This is an archive of the discontinued LLVM Phabricator instance.

[CodeGen] add rotate builtins
ClosedPublic

Authored by spatel on Aug 17 2018, 2:01 PM.

Details

Summary

This exposes the LLVM funnel shift intrinsics as more familiar bit rotation functions in clang (when both halves of a funnel shift are the same value, it's a rotate).

I think we're free to name these as we want because we're not copying gcc, but if there's some other existing art that we want to replicate, let me know.

The funnel shift intrinsics were added here:
D49242

With improved codegen in:
rL337966
rL339359

And basic IR optimization added in:
rL338218
rL340022

...so I think these should produce asm output that's equal or better to the multi-instruction alternatives using primitive C/IR ops.

In the motivating loop example from PR37387:
https://bugs.llvm.org/show_bug.cgi?id=37387#c7
...we get the expected 'rolq' x86 instructions if we substitute the rotate builtin into the source.

Diff Detail

Repository
rC Clang

Event Timeline

spatel created this revision.Aug 17 2018, 2:01 PM
rnk added a comment.Aug 17 2018, 2:10 PM

Do you mind updating the _rotl* and _rotr* intrinsics to use the same codegen? They're right above in the switch.

spatel updated this revision to Diff 161325.Aug 17 2018, 2:11 PM

Patch updated:
Fixed a docs typo.

In D50924#1204772, @rnk wrote:

Do you mind updating the _rotl* and _rotr* intrinsics to use the same codegen? They're right above in the switch.

Sure - I didn't know about those.

spatel updated this revision to Diff 161346.Aug 17 2018, 3:22 PM

Patch updated:
Update the existing Microsoft rotate builtins to also use the LLVM funnel shift intrinsics.

Looks ok to me.

rnk accepted this revision.Aug 17 2018, 4:00 PM

Thanks, looks good!

This revision is now accepted and ready to land.Aug 17 2018, 4:00 PM
This revision was automatically updated to reflect the committed changes.
spatel reopened this revision.Aug 19 2018, 6:58 AM

Reopening because I reverted at rL340136. Not sure yet what is causing the problem on those bots.

This revision is now accepted and ready to land.Aug 19 2018, 6:58 AM

Reopening because I reverted at rL340136. Not sure yet what is causing the problem on those bots.

The common trait for those failures appears to be that the host compiler is:
gcc version 7.3.1 20180130 (Red Hat 7.3.1-2) (GCC)

...but that compiler is crashing while compiling a file that this patch does not actually touch. Anyone know what our commit policy suggests for this situation?

This revision was automatically updated to reflect the committed changes.
spatel reopened this revision.Aug 19 2018, 9:24 AM

Reopening again because I reverted this again for the same reason at rL340138.

This revision is now accepted and ready to land.Aug 19 2018, 9:24 AM
This revision was automatically updated to reflect the committed changes.
spatel added a subscriber: lei.Aug 19 2018, 10:31 AM

And the 3rd time is the charm...
I have no explanation for why this works, but I removed the microsoft diffs, and now gcc doesn't crash.
cc'ing @lei as owner of the bots that have this problem:
http://lab.llvm.org:8011/builders/clang-ppc64be-linux/builds/22826/steps/build%20stage%201/logs/stdio

I'm not sure if I can repro that crash locally (requires PPC?). Maybe there's something useful that can be recovered from the bots to repro and file a gcc bug?