Page MenuHomePhabricator

ARM: Add ImmArg to intrinsics
ClosedPublic

Authored by arsenm on Feb 18 2019, 7:59 AM.

Details

Summary

I found these by asserting in clang for any GCCBuiltin that doesn't
require mangling and requires a constant for the builtin. This means
that intrinsics are missing which don't use GCCBuiltin, don't have
builtins defined in clang, or were missing the constant annotation in
the builtin definition.

Diff Detail

Event Timeline

arsenm created this revision.Feb 18 2019, 7:59 AM
rovka added a comment.Feb 20 2019, 2:24 AM

Can you explain why this is relevant only for GCC builtins that don't require mangling? Otherwise LGTM.

include/llvm/IR/IntrinsicsARM.td
317

I think these should also have immediate arguments, at least according to https://reviews.llvm.org/source/clang/browse/cfe/trunk/test/Sema/builtins-arm.c

Is there any reason why we should skip them? (I can see that they're just Intrinsic as opposed to GCCBuiltin, and the commit message says they wouldn't be included, but I don't really understand why you chose only GCCBuiltins).

arsenm marked an inline comment as done.Feb 20 2019, 5:53 AM
arsenm added inline comments.
include/llvm/IR/IntrinsicsARM.td
317

My assert method can only find the cases that use GCCBuiltin, All intrinsics with this constraint need it, but those require manually auditing

arsenm updated this revision to Diff 187584.Feb 20 2019, 7:20 AM
arsenm marked an inline comment as done.

Handled mentioned intrinsics

rovka accepted this revision.Feb 21 2019, 6:16 AM

Ok, thanks!

This revision is now accepted and ready to land.Feb 21 2019, 6:16 AM
arsenm closed this revision.Mar 14 2019, 6:45 AM

r356144