llvm dev - Jun 2011 - [LLVMdev] Mips backend -- Incorrect globaladdr/constpool address generation when bit 15 of address is set?

Hi All,
     In SVN head, MipsISelDAGToDAG.cpp has the following optimization:

   // Operand is a result from an ADD.
   if (Addr.getOpcode() == ISD::ADD) {
     // When loading from constant pools, load the lower address part in
     // the instruction itself. Example, instead of:
     //  lui $2, %hi($CPI1_0)
     //  addiu $2, $2, %lo($CPI1_0)
     //  lwc1 $f0, 0($2)
     // Generate:
     //  lui $2, %hi($CPI1_0)
     //  lwc1 $f0, %lo($CPI1_0)($2)
     if ((Addr.getOperand(0).getOpcode() == MipsISD::Hi ||
          Addr.getOperand(0).getOpcode() == ISD::LOAD) &&
         Addr.getOperand(1).getOpcode() == MipsISD::Lo) {
       SDValue LoVal = Addr.getOperand(1);
       if (dyn_cast<ConstantPoolSDNode>(LoVal.getOperand(0))) {
         Base = Addr.getOperand(0);
         Offset = LoVal.getOperand(0);
         return true;
       }
     }
   }

This optimization folds the low 16 bits of the global address into the 
load offset,
rather than generating a separate 'addiu' instruction.

To my understanding, the offset fields in MIPS load instructions are 
sign-extended,
not zero-extended, so this optimization will give the wrong answer if 
%lo($CPI1_0)
is, for example, 0xFFFF, because the address computed will be (r2-1), 
not (r2+65535).

It seems like you can only perform this optimization if you know the 
absolute address
(which you won't until link time), and if bit 15 of the address happens 
to be set,
you have to do some math to get the sign-extended offset to compute the 
right thing
(essentially, you'd have to "lui $2, (%hi(CPI1_0)+1)").

Is this a bug?

Thanks,
Matt

To close the loop, this was *not* a bug, %hi and %lo are manipulated in 
the linker
to point to the correct address.
-Matt

On 06/22/2011 05:48 PM, Matt Johnson wrote:
> Hi All,
>     In SVN head, MipsISelDAGToDAG.cpp has the following optimization:
>
>   // Operand is a result from an ADD.
>   if (Addr.getOpcode() == ISD::ADD) {
>     // When loading from constant pools, load the lower address part in
>     // the instruction itself. Example, instead of:
>     //  lui $2, %hi($CPI1_0)
>     //  addiu $2, $2, %lo($CPI1_0)
>     //  lwc1 $f0, 0($2)
>     // Generate:
>     //  lui $2, %hi($CPI1_0)
>     //  lwc1 $f0, %lo($CPI1_0)($2)
>     if ((Addr.getOperand(0).getOpcode() == MipsISD::Hi ||
>          Addr.getOperand(0).getOpcode() == ISD::LOAD) &&
>         Addr.getOperand(1).getOpcode() == MipsISD::Lo) {
>       SDValue LoVal = Addr.getOperand(1);
>       if (dyn_cast<ConstantPoolSDNode>(LoVal.getOperand(0))) {
>         Base = Addr.getOperand(0);
>         Offset = LoVal.getOperand(0);
>         return true;
>       }
>     }
>   }
>
> This optimization folds the low 16 bits of the global address into the 
> load offset,
> rather than generating a separate 'addiu' instruction.
>
> To my understanding, the offset fields in MIPS load instructions are 
> sign-extended,
> not zero-extended, so this optimization will give the wrong answer if 
> %lo($CPI1_0)
> is, for example, 0xFFFF, because the address computed will be (r2-1), 
> not (r2+65535).
>
> It seems like you can only perform this optimization if you know the 
> absolute address
> (which you won't until link time), and if bit 15 of the address 
> happens to be set,
> you have to do some math to get the sign-extended offset to compute 
> the right thing
> (essentially, you'd have to "lui $2, (%hi(CPI1_0)+1)").
>
> Is this a bug?
>
> Thanks,
> Matt