Page MenuHomePhabricator

[lanai] Add Lanai backend to clang driver
ClosedPublic

Authored by jpienaar on Feb 8 2016, 2:52 PM.

Details

Summary

Changes to clang to add Lanai backend. Adds a new target, ABI and toolchain.

General Lanai backend discussion on llvm-dev thread "[RFC] Lanai backend" (http://lists.llvm.org/pipermail/llvm-dev/2016-February/095118.html).

Diff Detail

Repository
rL LLVM

Event Timeline

jpienaar updated this revision to Diff 47252.Feb 8 2016, 2:52 PM
jpienaar retitled this revision from to [lanai] Add Lanai backend to clang driver.
jpienaar updated this object.
jpienaar updated this object.Feb 9 2016, 10:39 AM
jpienaar added a subscriber: cfe-commits.
eliben added a subscriber: eliben.Feb 9 2016, 1:23 PM

Clang-level tests?

lib/Basic/Targets.cpp
5873 ↗(On Diff #47252)

Maybe make this consistent with the next line comment?

lib/CodeGen/TargetInfo.cpp
6498 ↗(On Diff #47252)

Worth a comment? (I dont see thsi documented elsewhere)

6535 ↗(On Diff #47252)

Prefer nullptr comparison (here and elsewhere)

jpienaar updated this revision to Diff 47408.Feb 9 2016, 5:02 PM

Added clang-level tests.

jpienaar marked 3 inline comments as done.Feb 9 2016, 5:11 PM

Updated thanks.

lib/CodeGen/TargetInfo.cpp
6498 ↗(On Diff #47408)

We only had one DefaultNumRegisterParameters that we supported (DefaultNumRegisterParameters=4) so I just propagated the constant, removed the member variable and added a comment at its use.

eliben added inline comments.Feb 16 2016, 9:52 AM
lib/CodeGen/TargetInfo.cpp
6498 ↗(On Diff #47408)

SG, thanks

test/CodeGen/lanai-arguments.c
1 ↗(On Diff #47408)

why wasm triple?

jpienaar updated this revision to Diff 48119.Feb 16 2016, 3:57 PM
jpienaar updated this object.
jpienaar marked an inline comment as done.

Move mregparm check to Tools.cpp and correct triple used in test.

jpienaar added inline comments.Feb 16 2016, 3:58 PM
test/CodeGen/lanai-arguments.c
1 ↗(On Diff #47408)

Fixed, thanks.

jpienaar updated this object.Feb 24 2016, 9:32 AM
jpienaar updated this revision to Diff 49119.Feb 25 2016, 4:17 PM
jpienaar added reviewers: eliben, jyknight, chandlerc.

Updated preprocessor test init.c to use -match-full-lines.

jpienaar updated this revision to Diff 49688.Mar 2 2016, 4:49 PM

Removed unused variables in LanaiTargetInfo.

jpienaar updated this revision to Diff 49969.Mar 7 2016, 8:42 AM

Updated to use resetDataLayout.

Friendly ping.

majnemer added inline comments.
lib/CodeGen/TargetInfo.cpp
6592–6594 ↗(On Diff #49969)

Is this used? A quick grep didn't find any callers.

6622–6626 ↗(On Diff #49969)

Is floating point supported?

6636 ↗(On Diff #49969)

I think this can be unsigned SizeInRegs = alignTo(Size, 32U);

6656–6658 ↗(On Diff #49969)

I'd use const auto * here because EnumType is visible in the getAs`.

6660–6670 ↗(On Diff #49969)

I think this is equivalent to:

if (shouldUseInReg(Ty, State))
  return ABIArgInfo::getDirectInReg();

if (Ty->isPromotableIntegerType())
  return ABIArgInfo::getExtend();

return ABIArgInfo::getDirect();
test/CodeGen/lanai-arguments.c
2 ↗(On Diff #49969)

No need to use a check-prefix if there is only one FileCheck invocation.

jpienaar updated this revision to Diff 51806.Mar 28 2016, 10:58 AM
jpienaar marked 6 inline comments as done.

Removed unnecessary floating point classification (only integer is supported) and performed suggested cleanups.

Updated, thanks

lib/CodeGen/TargetInfo.cpp
6622–6626 ↗(On Diff #49969)

No, good point. I think we can remove this and introduce it again if floating point is supported.

chandlerc accepted this revision.Mar 28 2016, 12:53 PM
chandlerc edited edge metadata.

LGTM with the comments from David addressed and the comment below addressed.

lib/Basic/Targets.cpp
6000–6001 ↗(On Diff #51806)

Instead of a default, make this a covering switch? We get warnings that way if someone adds a CPU and doesn't add a case.

This revision is now accepted and ready to land.Mar 28 2016, 12:53 PM
echristo accepted this revision.Mar 28 2016, 1:01 PM
echristo added a reviewer: echristo.
echristo added a subscriber: echristo.

LGTM, thanks.

-eric

This revision was automatically updated to reflect the committed changes.
jpienaar marked an inline comment as done.