This is an archive of the discontinued LLVM Phabricator instance.

Add sqrt lowering from standard to ROCDL
ClosedPublic

Authored by akuegel on Dec 9 2020, 1:47 AM.

Details

Summary

Add a lowering for sqrt from standard dialect to ROCDL.

Diff Detail

Event Timeline

akuegel created this revision.Dec 9 2020, 1:47 AM
akuegel requested review of this revision.Dec 9 2020, 1:47 AM
herhut added a comment.Dec 9 2020, 1:52 AM

Can you add a test?

mlir/lib/Conversion/GPUToROCDL/LowerGpuOpsToROCDLOps.cpp
118

SqrtOp?

akuegel updated this revision to Diff 310464.Dec 9 2020, 2:00 AM

Fixed copy/paste bug.

mlir/lib/Conversion/GPUToROCDL/LowerGpuOpsToROCDLOps.cpp
118

Oops, copy/paste bug. Thanks for catching this :)

Working on a test now.

akuegel updated this revision to Diff 310465.Dec 9 2020, 2:14 AM

Add test.

I have copied the Sqrt test for the GPUToNVVM lowering and adapted it.

Frederik, it looks like there was an error on Windows during the pre-merge checks. Does that look familiar to the one you saw? Do you know if this is for real?

Ok, I just checked the recent presubmit results, and all seem to fail on Windows:
https://buildkite.com/llvm-project/premerge-checks/builds

So I think this is unrelated to my change :)

I saw the exact same failure and I'm pretty sure it is unrelated.

Looks good. Thanks!
We probably want this for rsqrt, too.

mlir/lib/Conversion/GPUToROCDL/LowerGpuOpsToROCDLOps.cpp
70–77

You may want to expose this like we did for NVVM here.
Can be a separate CL though

frgossen accepted this revision.Dec 9 2020, 6:36 AM
This revision is now accepted and ready to land.Dec 9 2020, 6:36 AM
akuegel added inline comments.Dec 10 2020, 12:43 AM
mlir/lib/Conversion/GPUToROCDL/LowerGpuOpsToROCDLOps.cpp
70–77

Thanks for the pointer :)
Yes, I think I will do this in a separate CL. Also Rsqrt in a separate CL.

This revision was landed with ongoing or failed builds.Dec 10 2020, 12:48 AM
This revision was automatically updated to reflect the committed changes.