Page MenuHomePhabricator

clang: Add ARCTargetInfo
Needs ReviewPublic

Authored by petecoup on Feb 8 2018, 1:41 PM.

Diff Detail

Event Timeline

petecoup created this revision.Feb 8 2018, 1:41 PM
Eugene.Zelenko added inline comments.
clang/lib/CodeGen/TargetInfo.cpp
8220

Please change to // namespace

petecoup updated this revision to Diff 133674.Feb 9 2018, 1:06 PM

Hello Eugene,

Thanks for taking a look.
Changed End anonymous namespace. -> namespace

petecoup marked an inline comment as done.Feb 9 2018, 1:06 PM

Hello Pete,

Thank you for upstreaming ARC target to clang! I checked it in couple with lldb - works well. So, the revision is good to me. But it seems, I have no rights to accept revisions yet...

Hi Tatyana,

Thanks for checking against lldb! I will try and find a couple of people who can review this as a clang change.

Pete

hintonda removed a subscriber: hintonda.Feb 9 2018, 2:47 PM

This is what I could note

clang/lib/Basic/Targets/ARC.h
26

Looks like unused member, while getTargetBuiltins returns 'None'

clang/lib/CodeGen/TargetInfo.cpp
8123

Better use '= default' instead of {}
And you even may use inheriting constructor here

8165

and here

Sorry, =default is not applicable in these cases, of course.

clang/lib/CodeGen/TargetInfo.cpp
8123

Here should be inheriting of base class constructor:

using DefaultABIInfo::DefaultABIInfo;

Hi Tatyana,

Thanks for taking a look here, and sorry for the delay.
I tried to follow the other target's usage patterns here.
I'm happy to update/change this...I'll add a couple of clang reviewers here to weigh in?

rjmccall added inline comments.Feb 14 2018, 10:25 AM
clang/lib/CodeGen/TargetInfo.cpp
8185

Please do go ahead and add the right logic for the C++ ABI in both this and the return-type classification.