[llvm] r259228 - ARM: don't mangle DAG constant if it has more than one use

[Hans Wennborg via llvm-commits]

Sounds good, go ahead and merge.

Thanks,
Hans

On Fri, Jan 29, 2016 at 12:17 PM, Tim Northover <t.p.northover at gmail.com> wrote:
> Hi Hans,
>
> Do you mind if I cherry-pick this over to 3.8? It's pretty safe fix
> for a codegen bug, and fairly surprising it hasn't already been
> spotted.
>
> Cheers.
>
> Tim.
>
> On 29 January 2016 at 11:18, Tim Northover via llvm-commits
> <llvm-commits at lists.llvm.org> wrote:
>> Author: tnorthover
>> Date: Fri Jan 29 13:18:46 2016
>> New Revision: 259228
>>
>> URL: http://llvm.org/viewvc/llvm-project?rev=259228&view=rev
>> Log:
>> ARM: don't mangle DAG constant if it has more than one use
>>
>> The basic optimisation was to convert (mul $LHS, $complex_constant) into
>> roughly "(shl (mul $LHS, $simple_constant), $simple_amt)" when it was expected
>> to be cheaper. The original logic checks that the mul only has one use (since
>> we're mangling $complex_constant), but when used in even more complex
>> addressing modes there may be an outer addition that can pick up the wrong
>> value too.
>>
>> I *think* the ARM addressing-mode problem is actually unreachable at the
>> moment, but that depends on complex assessments of the profitability of
>> pre-increment addressing modes so I've put a real check in there instead of an
>> assertion.
>>
>> Modified:
>>     llvm/trunk/lib/Target/ARM/ARMISelDAGToDAG.cpp
>>     llvm/trunk/test/CodeGen/ARM/shifter_operand.ll
>>
>> Modified: llvm/trunk/lib/Target/ARM/ARMISelDAGToDAG.cpp
>> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/ARM/ARMISelDAGToDAG.cpp?rev=259228&r1=259227&r2=259228&view=diff
>> ==============================================================================
>> --- llvm/trunk/lib/Target/ARM/ARMISelDAGToDAG.cpp (original)
>> +++ llvm/trunk/lib/Target/ARM/ARMISelDAGToDAG.cpp Fri Jan 29 13:18:46 2016
>> @@ -747,7 +747,7 @@ bool ARMDAGToDAGISel::SelectLdStSOReg(SD
>>
>>    // If Offset is a multiply-by-constant and it's profitable to extract a shift
>>    // and use it in a shifted operand do so.
>> -  if (Offset.getOpcode() == ISD::MUL) {
>> +  if (Offset.getOpcode() == ISD::MUL && N.hasOneUse()) {
>>      unsigned PowerOfTwo = 0;
>>      SDValue NewMulConst;
>>      if (canExtractShiftFromMul(Offset, 31, PowerOfTwo, NewMulConst)) {
>> @@ -1422,7 +1422,7 @@ bool ARMDAGToDAGISel::SelectT2AddrModeSo
>>
>>    // If OffReg is a multiply-by-constant and it's profitable to extract a shift
>>    // and use it in a shifted operand do so.
>> -  if (OffReg.getOpcode() == ISD::MUL) {
>> +  if (OffReg.getOpcode() == ISD::MUL && N.hasOneUse()) {
>>      unsigned PowerOfTwo = 0;
>>      SDValue NewMulConst;
>>      if (canExtractShiftFromMul(OffReg, 3, PowerOfTwo, NewMulConst)) {
>>
>> Modified: llvm/trunk/test/CodeGen/ARM/shifter_operand.ll
>> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/CodeGen/ARM/shifter_operand.ll?rev=259228&r1=259227&r2=259228&view=diff
>> ==============================================================================
>> --- llvm/trunk/test/CodeGen/ARM/shifter_operand.ll (original)
>> +++ llvm/trunk/test/CodeGen/ARM/shifter_operand.ll Fri Jan 29 13:18:46 2016
>> @@ -239,3 +239,20 @@ define void @test_well_formed_dag(i32 %i
>>    store i32 %add, i32* %addr
>>    ret void
>>  }
>> +
>> +define { i32, i32 } @test_multi_use_add(i32 %base, i32 %offset) {
>> +; CHECK-LABEL: test_multi_use_add:
>> +; CHECK-THUMB: movs [[CONST:r[0-9]+]], #28
>> +; CHECK-THUMB: movt [[CONST]], #1
>> +
>> +  %prod = mul i32 %offset, 65564
>> +  %sum = add i32 %base, %prod
>> +
>> +  %ptr = inttoptr i32 %sum to i32*
>> +  %loaded = load i32, i32* %ptr
>> +
>> +  %ret.tmp = insertvalue { i32, i32 } undef, i32 %sum, 0
>> +  %ret = insertvalue { i32, i32 } %ret.tmp, i32 %loaded, 1
>> +
>> +  ret { i32, i32 } %ret
>> +}
>>
>>
>> _______________________________________________
>> llvm-commits mailing list
>> llvm-commits at lists.llvm.org
>> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits