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

[Joerg Sonnenberger via llvm-commits]

On Tue, Sep 29, 2015 at 04:47:10PM +0000, James Y Knight wrote:
> 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?

Not easily. The logic in matchSparcAsmModifiers also has to deal with
GOT references, which doesn't apply to the "set" case. Splitting the
logic on the other hand seems to make it more complicated, not less.

> ================
> 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.

Not completely, the handling of GOT was different before. Using the same
input for both cases makes it easier to verify the behavior with GAS.


> ================
> 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.

Sure, will do.

Joerg