[PATCH] D13173: [SPARC] Improve PIC vs non-PIC handling

[James Y Knight via llvm-commits]

jyknight added inline comments.

================
Comment at: lib/Target/Sparc/AsmParser/SparcAsmParser.cpp:470
@@ -467,1 +469,3 @@
+  // In PIC mode, the expression refers to a symbol in the GOT.
+  // The conditions correspond to the checks in matchSparcAsmModifiers.
   if (!IsEffectivelyImm13) {
----------------
"set _GLOBAL_OFFSET_TABLE_" should be emitted with a PC22/PC10 reloc to match, too. Can you extract the kind conversion logic to a separate function and just call it?

================
Comment at: lib/Target/Sparc/AsmParser/SparcAsmParser.cpp:1088
@@ -1079,2 +1087,3 @@
 
-  bool isPIC = getContext().getObjectFileInfo()->getRelocM() == Reloc::PIC_;
+  // When in PIC mode, "%lo(...)" and "%hi(...)" work slightly different.
+  // If the expression refers contains _GLOBAL_OFFSETE_TABLE, it is
----------------
"work slightly differently" seems rather understating the extent of the insanity. :)

================
Comment at: test/MC/Sparc/sparc-pic.s:2
@@ -2,1 +1,3 @@
+! RUN: llvm-mc %s -arch=sparcv9 --relocation-model=pic -filetype=obj | llvm-readobj -r | FileCheck --check-prefix=PIC %s
+! RUN: llvm-mc %s -arch=sparcv9 --relocation-model=static -filetype=obj | llvm-readobj -r | FileCheck --check-prefix=NOPIC %s
 
----------------
The nonpic cases are covered by sparc-relocations.s already I think? Guess it's fine to restate them here tho.

================
Comment at: test/MC/Sparc/sparc-pic.s:49
@@ -36,4 +48,3 @@
         add %i1, %o7, %i1
-        sethi %hi(AGlobalVar), %i2
-        add %i2, %lo(AGlobalVar), %i2
+        set AGlobalVar, %i2
         ldx [%i1+%i2], %i3
----------------
Might be nice to keep both cases since they're supposed to end up the same.


http://reviews.llvm.org/D13173