[PATCH] D70243: Lowering CPI/JTI/BA to assembly

[Sean Fertile via Phabricator via llvm-commits]

sfertile marked an inline comment as done.
sfertile added inline comments.


================
Comment at: llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp:1757
 
+      if (getSubtargetInfo().getTargetTriple().getObjectFormat() ==
+	  Triple::XCOFF) {
----------------
Xiangling_L wrote:
> Xiangling_L wrote:
> > jasonliu wrote:
> > > sfertile wrote:
> > > > Nit: Formatting.
> > > We could call member function of triple instead of raw comparison:
> > > TM.getTargetTriple().isOSAIX or TM.getTargetTriple().isOSBinFormatXCOFF
> > > Same for all the raw comparison below. 
> > Sorry, but can you point out the formatting issue more explicitly?  And the formatting here is got from `clang-format`.
> Thanks, I will update it.
Its fixed now, but  phabricator was showing the code inside the if block was indented less then the if statement itself.


================
Comment at: llvm/lib/CodeGen/MachineModuleInfo.cpp:125
+        Context.lookupSymbol("." + Entry.Fn->getName());
+    assert(FnEntryPointSym && "The function entry pointer symbol should has"
+		              " already been initialized.");
----------------
minor nit: `has` --> `have`.


================
Comment at: llvm/lib/CodeGen/TargetLoweringObjectFileImpl.cpp:1875
+          " yet.");
+  assert (!F.getComdat() && "Comdat not supported on XCOFF yet.");
+  //TODO: Enable emiting jump table to unique sections when we support it.
----------------
Minor nit: IIUC then Comdats aren't supported by the XCOFF specification. The `yet` at the ned makes it sound to me like its something supported but not implemented in LLVM yet.


================
Comment at: llvm/test/CodeGen/PowerPC/aix-lower-constant-pool-index.ll:87
+; CHECK: .toc
+; CHECK-NOT: .tc
----------------
I don't understand why we have a check-not in each of the tests for `.tc`. Is it because we haven't implemented the emission of the toc entries yet and its a reminder to fixup the test when we do implement that?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D70243/new/

https://reviews.llvm.org/D70243