Page MenuHomePhabricator

[COFF, ARM64] Add MS builtins __dmb, __dsb, __isb
ClosedPublic

Authored by mgrang on Jul 31 2017, 1:27 PM.

Diff Detail

Event Timeline

mgrang created this revision.Jul 31 2017, 1:27 PM
mstorsjo edited edge metadata.Jul 31 2017, 1:40 PM

Out of curiousity (I'm not familiar enough with this aspect of LLVM internals) - what practical difference does this make - wouldn't the existing llvm level intrinsic be usable as such already? (I do see that the same is done for __builtin_arm_dmb and __dmb etc for ARM already, so I'm sure it's right - just curious where it makes a difference.)

GCCBuiltin/MSBuiltin emit tables which get used by clang IR generation. See Intrinsic::getIntrinsicForGCCBuiltin/Intrinsic::getIntrinsicForMSBuiltin.

GCCBuiltin/MSBuiltin emit tables which get used by clang IR generation. See Intrinsic::getIntrinsicForGCCBuiltin/Intrinsic::getIntrinsicForMSBuiltin.

Thanks for the explanation!

This change looks ok to me. I guess this is a change that isn't too easy to add tests for so I guess it's ok as is?

compnerd edited edge metadata.Aug 1 2017, 8:12 PM

Might be a good idea to add tests for the IR itself that we lower it properly.

efriedma accepted this revision.Aug 2 2017, 12:26 PM

LGTM.

There aren't any useful tests to add here because the generated tables are only used by clang.

@compnerd We already have tests for llvm.aarch64.dmb etc.

This revision is now accepted and ready to land.Aug 2 2017, 12:26 PM
This revision was automatically updated to reflect the committed changes.