[PATCH] D37584: Add target-independent backend modifications for pagerando

[Peter Smith via Phabricator via llvm-commits]

peter.smith added inline comments.


================
Comment at: lib/CodeGen/AsmPrinter/AsmPrinter.cpp:441
+
+  // TODO(sjc): refactor this with redundant code in EmitConstantPool()
+  unsigned Align = CPE.getAlignment();
----------------
I think that this is worth doing now, when reviewing I had to double check that the duplication was computing the same thing.


================
Comment at: lib/CodeGen/AsmPrinter/AsmPrinter.cpp:1733
 }
 
+void AsmPrinter::EmitPOT() {
----------------
Given that POT is not as widely documented or known as the GOT, I think it would be helpful to put a comment describing what it is. I mention it here as we need to know the structure of it in order to emit it. There could be better places though.


================
Comment at: lib/CodeGen/AsmPrinter/AsmPrinter.cpp:1751
+  auto *GOTSym = OutContext.getOrCreateSymbol("_GLOBAL_OFFSET_TABLE_");
+  EmitLabelReference(GOTSym, PtrSize);
+
----------------
It may be worth taking some care over using _GLOBAL_OFFSET_TABLE_. It is a linker convention to define _GLOBAL_OFFSET_TABLE_ to be the base of the .GOT but it might not always hold. For example earlier versions of lld set _GLOBAL_OFFSET_TABLE_ to 0 as the relocation types used by the compiler didn't look up the symbol value. Some backends (not Arm or AArch64) recognize the symbol and do something special with it as well.

I think what you have will work for gold on Arm and AArch64 but it may be worth letting targets override it with a "get me the got base" relocation. 

  


https://reviews.llvm.org/D37584