[CODEGEN][AOCL] Add math intrinsic rules #1653
Conversation
|
@tmoreau89 @nishi-t @Ktabata Can you review this PR? |
|
Removing target device name looks good to me. |
src/codegen/codegen_aocl.cc
Outdated
| for (std::string option : target->options()) { | ||
| if (option == "-mattr=emulator") { | ||
| cmd += " -march=emulator"; | ||
| } | ||
| } | ||
| cmd += " -board=" + target->device_name; |
There was a problem hiding this comment.
This works on my machine.
topi/tests/python/test_topi_math.py
Outdated
| @@ -39,7 +39,8 @@ def check_device(device): | |||
| foo(a, b) | |||
| np.testing.assert_allclose(b.asnumpy(), b_np, rtol=1e-5, atol=1e-5) | |||
|
|
|||
| for device in ['cuda', 'opencl', 'metal', 'rocm', 'vulkan', 'llvm', 'nvptx', 'sdaccel']: | |||
| for device in ['cuda', 'opencl', 'metal', 'rocm', 'vulkan', 'llvm', 'nvptx', 'sdaccel', | |||
| 'aocl -mattr=emulator']: | |||
There was a problem hiding this comment.
would it make sense to rename the device string to a something like aocl_emu? keeping in mind that there are different forms of emulation (sw vs. hw)
|
Overall the changes look good. My only question touches on the way we pass the emulation attribute to the AOCL code gen backend. One way is to use a target option as we do, but another would be to simply add another target string like aocl_emu which makes it easier to parse targets strings. |
|
@tmoreau89 Thanks for your comment. Adding a target string for emulation sounds good to me. I've pushed a commit for that, can you check it out? |
|
thank you, I think having an emulation target makes sense in this context. Does the aocl support different types of emulation (sw vs. hw as with sdaccel)? |
|
@tmoreau89 AFAIK, AOCL supports only software emulation. No need to care about different kind of emulation. |
There was a problem hiding this comment.
@kazum Could you fix the aocl document in accordance with the change?
https://github.com/dmlc/tvm/blob/master/docs/deploy/aocl_fpga.md
-mattr=emulator will be invalid
|
If we will support different emulation types for sdaccel by naming (e.g. |
|
indeed some folks might care about the distinction between hardware and software emulation, therefore it might be good to explicitly specify it since sdaccel offers both modes (to avoid any ambiguity). Since execution time can vary by 1-2 orders of magnitude from software to hardware emulation, it'll be good to have explicit control by changing the target name. |
|
@tmoreau89 @nishi-t Thanks, I’ve addressed your comments. I think of introducing sdaccel_sw_emu and sdaccel_hw_emu in a future PR, too. :) |
|
LGTM |
* [CODEGEN][AOCL] Add math intrinsic rules * introduce aocl_emu target for AOCL emulation * rename aocl_emu with aocl_sw_emu * update docs
* [CODEGEN][AOCL] Add math intrinsic rules * introduce aocl_emu target for AOCL emulation * rename aocl_emu with aocl_sw_emu * update docs
This PR adds math intrinsic rules for AOCL.
In addition, to simplify an AOCL target name for emulation, I made the
-deviceparameter of the AOCL target optional.