This is an archive of the discontinued LLVM Phabricator instance.

Fix arm_neon.h and arm_fp16.h generation for compiling with std=c89
ClosedPublic

Authored by dnsampaio on Sep 5 2018, 7:09 AM.

Details

Summary

The inline attribute is not valid for C standard 89. Replace the argument in the generation of header files with __inline, as well adding tests for both header files.

Diff Detail

Repository
rL LLVM

Event Timeline

dnsampaio created this revision.Sep 5 2018, 7:09 AM
t.p.northover accepted this revision.Sep 5 2018, 7:36 AM
t.p.northover added a subscriber: t.p.northover.

Looks good to me.

This revision is now accepted and ready to land.Sep 5 2018, 7:36 AM
This revision was automatically updated to reflect the committed changes.
joerg added a subscriber: joerg.Sep 5 2018, 8:25 AM
joerg added inline comments.
cfe/trunk/utils/TableGen/NeonEmitter.cpp
2412

If you want to change it, at least change it properly to use inline.

2521

Same.

dnsampaio added inline comments.Sep 6 2018, 3:02 AM
cfe/trunk/utils/TableGen/NeonEmitter.cpp
2412

Sorry, I don't get the suggestion. Do you mean test if it is C89 and use __inline, else, use inline?

dnsampaio updated this revision to Diff 164196.Sep 6 2018, 5:59 AM

Fix test march triple.

dnsampaio updated this revision to Diff 164199.Sep 6 2018, 6:07 AM
dmgreen added a subscriber: dmgreen.Sep 6 2018, 6:29 AM

These new changes look good to me.

If you are updating things like this, it's often better to create a new Phab review so it's easier to see it's a new thing (or, in cases like this where the changes are simple and just test updates, they often don't need review). Either way, LGTM. Thanks for the test fix.

jgreenhalgh added inline comments.
cfe/trunk/utils/TableGen/NeonEmitter.cpp
2412

I think the suggestion was unintentionally rendered as an underline by phab; and was supposed to be

__inline__
joerg added a comment.Sep 6 2018, 12:53 PM

Correct. The protected name is double underscore as both suffix and prefix.

Correct. The protected name is double underscore as both suffix and prefix.

Ok, fixed. Cheers.