Skip to content

Conversation

@xqdan
Copy link
Contributor

@xqdan xqdan commented Oct 23, 2018

Thanks for contributing to TVM! Please refer to guideline https://docs.tvm.ai/contribute/ for useful information and tips. After the pull request is submitted, please request code reviews from Reviewers.

@siju-samuel @PariksheetPinjari909 @Huyuwei please review, thanks!

"""
return call_pure_intrin(x.dtype, "popcount", x)

def mod(x, y):
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

rename to fmod as we already have Mod in the ir node

return ir::Reduce::make(combiner, {source}, rdom, make_const(Bool(1), true), 0);
}

Expr mod(Expr x, Expr y) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

rename to fmod

check_device("cuda", "llvm")
check_device("vulkan")

def test_mod():
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

test_fmod

test_add()
test_log_pow_llvm()
test_popcount()
test_mod()
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

test_mod-> test_fmod


Expr fmod(Expr x, Expr y) {
BinaryOpMatchTypes(x, y);
CHECK(x.type().is_float()) << "mod only applies to float";
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fmod

return call_pure_intrin(x.dtype, "popcount", x)

def fmod(x, y):
"""Take mod cast of input x and y
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggest to make the docstring slightly detailed.

@tqchen
Copy link
Member

tqchen commented Oct 25, 2018

please also check other backends(vulkan, opencl, metal, rocm, nvptx) to add this function, you might need to lookup the specific reference of each backend

@tqchen tqchen added the status: need update need update based on feedbacks label Oct 25, 2018
@xqdan
Copy link
Contributor Author

xqdan commented Oct 27, 2018

I've checked a bit, but I don't find the right place for Vulkan, could you guys give me a link?
also another concern, notice that there are only gpu and cpu on CI, so if I checkin for other backend, since I have no env in my computer, how can I verify it?
BTW, these are links I found for other backends, also take a look and confirm, thanks!

openCL: yes. https://manpages.debian.org/testing/opencl-1.2-man-doc/fmod.3clc.en.html
metal: yes. https://developer.apple.com/metal/Metal-Shading-Language-Specification.pdf
rocm: yes. https://rocm-documentation.readthedocs.io/en/latest/ROCm_Compiler_SDK/ocml.html?highlight=fmod
nvptx: no. https://github.com/llvm-mirror/clang/blob/master/include/clang/Basic/BuiltinsNVPTX.def

@tqchen
Copy link
Member

tqchen commented Oct 27, 2018

@xqdan
Copy link
Contributor Author

xqdan commented Oct 27, 2018

@tqchen thanks, I mean the link of vulkan reference doc

@tqchen
Copy link
Member

tqchen commented Oct 28, 2018

These macros are defined in #include <GLSL.std.450.h> (they are not part of Vulkan doc, but more of GLSL standard)

@xqdan
Copy link
Contributor Author

xqdan commented Oct 29, 2018

@tqchen thanks
Vulkan: no fmod https://www.khronos.org/registry/spir-v/specs/1.0/GLSL.std.450.html

We can open a thread on discussion to record the reference links so that others can get them easily

@tqchen tqchen merged commit 32f158e into apache:master Oct 29, 2018
@tqchen
Copy link
Member

tqchen commented Oct 29, 2018

Thanks @xqdan @siju-samuel this is now merged. I agree that having a documented page in the docs/dev might be helpful for developers

@tqchen tqchen added status: accepted and removed status: need review status: need update need update based on feedbacks labels Oct 29, 2018
eqy pushed a commit to eqy/tvm that referenced this pull request Oct 29, 2018
FrozenGene pushed a commit to FrozenGene/tvm that referenced this pull request Dec 27, 2018
wweic pushed a commit to neo-ai/tvm that referenced this pull request Feb 20, 2019
wweic pushed a commit to neo-ai/tvm that referenced this pull request Feb 20, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants