[PATCH] D17011: [lanai] Add Lanai backend.

[Jacques Pienaar via llvm-commits]

jpienaar added a comment.

Updated and submitted.

Thanks for the good suggestions,

Jacques


================
Comment at: lib/Target/Lanai/AsmParser/LanaiAsmParser.cpp:453-457
@@ +452,7 @@
+    assert(N == 1 && "Invalid number of operands!");
+    if (const MCConstantExpr *ConstExpr = dyn_cast<MCConstantExpr>(getImm())) {
+      Inst.addOperand(MCOperand::createImm(ConstExpr->getValue() & 0xffff));
+    } else {
+      assert(false && "Operand type not supported.");
+    }
+  }
----------------
echristo wrote:
> LLVM style is no braces around single if/if-else statements. Going to be a bit of churn to fix though in your patch.
Used a ASM matcher to look for more of these.

================
Comment at: lib/Target/Lanai/AsmParser/LanaiAsmParser.cpp:756
@@ +755,3 @@
+
+unsigned AluWithPrePost(unsigned AluCode, bool PreOp, bool PostOp) {
+  if (PreOp)
----------------
t.p.northover wrote:
> Global-scope functions should be static to avoid exporting unnecessary symbols unless you really expect them to be accessed from elsewhere (this is just the first one I noticed).
Fixed thanks.

================
Comment at: lib/Target/Lanai/LanaiISelDAGToDAG.cpp:262-263
@@ +261,4 @@
+  switch (ConstraintCode) {
+  case InlineAsm::Constraint_o: // offsetable        ??
+  case InlineAsm::Constraint_v: // not offsetable    ??
+  default:
----------------
echristo wrote:
> Are you actually using these in a way that you should do something with them?
No, removed. Thanks

================
Comment at: lib/Target/Lanai/LanaiSubtarget.h:63
@@ +62,3 @@
+  const LanaiTargetLowering *getTargetLowering() const override {
+    return &target_lowering_info_;
+  }
----------------
echristo wrote:
> In general: What's with the underscores? :) Please follow the coding convention here (and everywhere).
Missed these.

================
Comment at: lib/Target/Lanai/LanaiTargetObjectFile.cpp:121
@@ +120,3 @@
+                                                        unsigned &Align) const {
+  if (isConstantInSmallSection(DL, C, *TM))
+    return SmallDataSection;
----------------
echristo wrote:
> ... and so doesn't need to be passed here, which means you don't need it on the class as every other function passes it down.
Good catch thanks.


Repository:
  rL LLVM

http://reviews.llvm.org/D17011