[PATCH] WinCOFF: Transform IR expressions featuring __ImageBase into image relative relocations

Nick Lewycky nlewycky at google.com
Mon Jan 13 20:39:54 PST 2014


  Someone else should look at the Target/* changes but I'm happy with the IR-level design and implementation.


================
Comment at: lib/Target/X86/X86TargetObjectFile.cpp:73
@@ +72,3 @@
+                  !GVRHS->hasSection() && GVRHS->hasExternalLinkage() &&
+                  GVRHS->hasName() && GVRHS->getName() == "__ImageBase") {
+                return MCSymbolRefExpr::Create(
----------------
Optional: hasName() && getName() == X is redundant, getName() returns StringRef("", 0) when hasName is false.


================
Comment at: test/MC/COFF/ir-to-imgrel.ll:1
@@ +1,2 @@
+; RUN: llc -mtriple=x86_64-pc-win32 %s   -o - | FileCheck %s --check-prefix=X64
+
----------------
Three spaces before -o ?

================
Comment at: test/MC/COFF/ir-to-imgrel.ll:3
@@ +2,3 @@
+
+ at __ImageBase = external global i8
+
----------------
I'm morbidly curious what happens when someone attempts to load or store this value. I guess the answer is "don't care"?

================
Comment at: lib/Target/X86/X86TargetObjectFile.cpp:60
@@ +59,3 @@
+    const ConstantExpr *CE, Mangler *Mang) const {
+  if (const SubOperator *Sub = dyn_cast<SubOperator>(CE))
+    if (const PtrToIntOperator *SubLHS =
----------------
Optional: how do you feel about:
  const SubOperator *Sub = dyn_cast<SubOperator>(CE);
  if (!Sub) return 0;
  const PtrToIntOperator *SubLHS = dyn_cast<PtrToIntOperator>(Sub->getOperand(0));
  if (!SubLHS) return 0;
  ...
form? The nesting level is ridiculous.

================
Comment at: include/llvm/Target/TargetLoweringObjectFile.h:33
@@ -32,2 +32,3 @@
   class GlobalValue;
+  class ConstantExpr;
   class TargetMachine;
----------------
Ow my sort order.


http://llvm-reviews.chandlerc.com/D2523

BRANCH
  IMAGEREL

ARCANIST PROJECT
  llvm



More information about the llvm-commits mailing list