[PATCH] D157907: [NFC] Refactor X86TargetLowering::getGlobalWrapperKind()

[Reid Kleckner via Phabricator via llvm-commits]

rnk added inline comments.


================
Comment at: llvm/lib/Target/X86/X86ISelLowering.cpp:18246
+  Result =
+      DAG.getNode(getGlobalWrapperKind(nullptr, OpFlag), DL, PtrVT, Result);
   // With PIC, the address is actually $g + Offset.
----------------
Are there more no-operand call sites? It seems that now they will do the wrong thing in the large code model. Should we remove the optional arguments to mandate that all sites pass OpFlag?


================
Comment at: llvm/lib/Target/X86/X86Subtarget.cpp:326
   // Determine the PICStyle based on the target selected.
-  if (!isPositionIndependent())
+  if (!isPositionIndependent() || TM.getCodeModel() == CodeModel::Large)
     setPICStyle(PICStyles::Style::None);
----------------
This is subtle, can you expand on why we use the "None" PIC style for the Large PIC code model? Clearly, the ISel works out correctly regardless, but it's surprising. Would it make more sense to have a PICStyles::Style::LargePIC?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D157907