[PATCH] D47211: [X86] Implement more of x86-64 large and medium PIC code models

[Reid Kleckner via Phabricator via llvm-commits]

rnk reopened this revision.
rnk marked an inline comment as done.
rnk added a subscriber: Jianping.
rnk added inline comments.


================
Comment at: llvm/trunk/lib/Target/X86/X86ISelDAGToDAG.cpp:946
   CodeModel::Model M = TM.getCodeModel();
-  if (Subtarget->is64Bit() && M != CodeModel::Small && M != CodeModel::Kernel)
+  if (Subtarget->is64Bit() && M != CodeModel::Small && M != CodeModel::Kernel &&
+      !(M == CodeModel::Medium && IsRIPRel))
----------------
echristo wrote:
> That's a lot of negatives :)
> 
> Also doesn't entirely look like the comment describes what's going on here - there's no comment about medium and riprel here. Otherwise you should be able to just use "M == CodeModel:Large" or something?
> 
> Thoughts?
Yeah, I think I wrote the comment, and then edited the code so that it was stale. @Jianping made the same change in D48417, so I think it's right.


================
Comment at: llvm/trunk/lib/Target/X86/X86InstrInfo.cpp:12726
       // uses RIP relative addressing.
-      if (STI.is64Bit())
+      if (STI.is64Bit() && (TM->getCodeModel() == CodeModel::Small ||
+                            TM->getCodeModel() == CodeModel::Kernel))
----------------
echristo wrote:
> Comment update?
I actually did update this one, but it missed the phab upload: rL335298 I'll be in the reland.


================
Comment at: llvm/trunk/lib/Target/X86/X86TargetMachine.cpp:159
 static Reloc::Model getEffectiveRelocModel(const Triple &TT,
+                                           bool JIT,
                                            Optional<Reloc::Model> RM) {
----------------
echristo wrote:
> Not sure I understand this change?
Many JIT tests use the large code model because object file sections may be spread out across memory. They don't need PIC codegen, though, since they are running locally in process memory and they don't need to be relocated. This was the cleanest way I could come up with to make them default to static codegen instead of PIC codegen.

However, that didn't save me from (Linux only) tests that explicitly exercise the PIC model:
llvm/test/ExecutionEngine/MCJIT/eh-lg-pic.ll

And I guess I broke a bunch of Darwin stuff :(
https://reviews.llvm.org/P8087
http://green.lab.llvm.org/green/job/clang-stage1-cmake-RA-incremental/49895/consoleFull

I've fixed the ELF MCJIT, but I'm not sure what to do about the macho things.


Repository:
  rL LLVM

https://reviews.llvm.org/D47211