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

[David Majnemer]

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

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

================
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
+
----------------
Nick Lewycky wrote:
> Three spaces before -o ?
Addressed.

================
Comment at: test/MC/COFF/ir-to-imgrel.ll:3
@@ +2,3 @@
+
+ at __ImageBase = external global i8
+
----------------
Nick Lewycky wrote:
> I'm morbidly curious what happens when someone attempts to load or store this value. I guess the answer is "don't care"?
We refuse to form an image relative relocation because they've done a weird thing.

The code tries pretty hard to match on a very specific form.

================
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 =
----------------
Nick Lewycky wrote:
> 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.
Addressed, switched to the early return form.


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

BRANCH
  IMAGEREL

ARCANIST PROJECT
  llvm