[llvm] r210414 - Do materialize for floating point

Alp Toker alp at nuanti.com
Sun Jun 8 02:40:01 PDT 2014


On 08/06/2014 11:42, Reed Kotler wrote:
> The summary area in the putback comment was messed up but the patch is 
> fine.
> I'm not very good with phabricator yet.

Reed, I understand that your commit log was messed up but that's not the 
actual problem.

I've reverted the change in r210424 with a full explanation and will 
follow up shortly to the list.

Alp.


>
> I don't understand your concerns.
>
> You should look at the other fast-isel implementations, i.e. ppc, arm 
> and x86.
>
> This one is almost identical.
>
> All the implementations are all essentially the same.
>
> You are reading too much into the comment "creates lot of core 
> infrastructure".
> It's not "core infrastructure" for llvm in general.
>
> This comment actually applied to an earlier commit. I have a lot of 
> patches backed up for
> review and phabricator was seeing earlier comments and I should have 
> deleted them
> but forgot to.
>
> The only thing this patch does is to add the additional functionality 
> for materializing
> floats.
>
> Reed
>
> On 06/08/2014 12:11 AM, Alp Toker wrote:
>> Hi Daniel,
>>
>> I'm concerned about the quality of your review...
>>
>>
>> On 08/06/2014 06:30, Reed Kotler wrote:
>>> Author: rkotler
>>> Date: Sat Jun  7 22:30:32 2014
>>> New Revision: 210414
>>>
>>> URL: http://llvm.org/viewvc/llvm-project?rev=210414&view=rev
>>> Log:
>>> Do materialize for floating point
>>>
>>> Summary:
>>> start to do simple constants
>>>
>>> finish simplestore
>>>
>>> add test case
>>>
>>> format
>>>
>>> Merge branch 'master' into 1756_8
>>>
>>> Add basic functionality for assignment of ints. This creates a lot 
>>> of core infrastructure in which to add, with little effort, quite a 
>>> bit more to mips fast-isel
>>
>> A patch that "creates lot of core infrastructure" should have been 
>> reviewed on-list.
>>
>>>
>>> Merge branch 'master' into 1756_8
>>>
>>> Add basic functionality for assignment of ints. This creates a lot 
>>> of core infrastructure in which to add, with little effort, quite a 
>>> bit more to mips fast-isel
>>>
>>> in progress
>>>
>>> finish integer materialize
>>>
>>> test cases
>>>
>>> test cases
>>>
>>> in progress
>>>
>>> Finish up fast-isel materialize for ints.
>>>
>>> Finish materialize for ints
>>>
>>> test cases
>>>
>>> simplestorei.ll
>>>
>>> Merge branch 'master' into 1756_8
>>>
>>> fix fp constants for fast-isel
>>>
>>> Merge branch '1758_1' of dmz-portal.mips.com:llvm into 1758_1
>>>
>>> in progress
>>>
>>> lastest for fp materialization
>>>
>>> clean up
>>>
>>> Merge branch 'master' into 1758_1
>>>
>>> formatting
>>>
>>> add test case
>>>
>>> finish test case
>>>
>>> Merge branch 'master' into 1758_2
>>>
>>> Test Plan:
>>> simplestore.ll
>>>
>>> simplestore.ll
>>>
>>> Reviewers: dsanders
>>>
>>> Reviewed By: dsanders
>>
>> ?
>>
>>>
>>> Differential Revision: http://reviews.llvm.org/D3659
>>
>> If I'm reading correctly, the quality issues were identified and just 
>> ignored to get a final LGTM and commit.
>>
>> What's going on here?
>>
>> Alp.
>>
>>
>>>
>>> Added:
>>>      llvm/trunk/test/CodeGen/Mips/Fast-ISel/simplestorefp1.ll
>>> Modified:
>>>      llvm/trunk/lib/Target/Mips/MipsFastISel.cpp
>>>
>>> Modified: llvm/trunk/lib/Target/Mips/MipsFastISel.cpp
>>> URL: 
>>> http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/Mips/MipsFastISel.cpp?rev=210414&r1=210413&r2=210414&view=diff
>>> ============================================================================== 
>>>
>>> --- llvm/trunk/lib/Target/Mips/MipsFastISel.cpp (original)
>>> +++ llvm/trunk/lib/Target/Mips/MipsFastISel.cpp Sat Jun  7 22:30:32 
>>> 2014
>>> @@ -167,9 +167,14 @@ bool MipsFastISel::EmitStore(MVT VT, uns
>>>     //
>>>     // more cases will be handled here in following patches.
>>>     //
>>> -  if (VT != MVT::i32)
>>> +  if (VT == MVT::i32)
>>> +    EmitInstStore(Mips::SW, SrcReg, Addr.Base.Reg, Addr.Offset);
>>> +  else if (VT == MVT::f32)
>>> +    EmitInstStore(Mips::SWC1, SrcReg, Addr.Base.Reg, Addr.Offset);
>>> +  else if (VT == MVT::f64)
>>> +    EmitInstStore(Mips::SDC1, SrcReg, Addr.Base.Reg, Addr.Offset);
>>> +  else
>>>       return false;
>>> -  EmitInstStore(Mips::SW, SrcReg, Addr.Base.Reg, Addr.Offset);
>>>     return true;
>>>   }
>>>   @@ -229,6 +234,22 @@ bool MipsFastISel::TargetSelectInstructi
>>>   }
>>>     unsigned MipsFastISel::MaterializeFP(const ConstantFP *CFP, MVT 
>>> VT) {
>>> +  int64_t Imm = CFP->getValueAPF().bitcastToAPInt().getZExtValue();
>>> +  if (VT == MVT::f32) {
>>> +    const TargetRegisterClass *RC = &Mips::FGR32RegClass;
>>> +    unsigned DestReg = createResultReg(RC);
>>> +    unsigned TempReg = Materialize32BitInt(Imm, &Mips::GPR32RegClass);
>>> +    EmitInst(Mips::MTC1, DestReg).addReg(TempReg);
>>> +    return DestReg;
>>> +  } else if (VT == MVT::f64) {
>>> +    const TargetRegisterClass *RC = &Mips::AFGR64RegClass;
>>> +    unsigned DestReg = createResultReg(RC);
>>> +    unsigned TempReg1 = Materialize32BitInt(Imm >> 32, 
>>> &Mips::GPR32RegClass);
>>> +    unsigned TempReg2 =
>>> +        Materialize32BitInt(Imm & 0xFFFFFFFF, &Mips::GPR32RegClass);
>>> +    EmitInst(Mips::BuildPairF64, 
>>> DestReg).addReg(TempReg2).addReg(TempReg1);
>>> +    return DestReg;
>>> +  }
>>>     return 0;
>>>   }
>>>
>>> Added: llvm/trunk/test/CodeGen/Mips/Fast-ISel/simplestorefp1.ll
>>> URL: 
>>> http://llvm.org/viewvc/llvm-project/llvm/trunk/test/CodeGen/Mips/Fast-ISel/simplestorefp1.ll?rev=210414&view=auto
>>> ============================================================================== 
>>>
>>> --- llvm/trunk/test/CodeGen/Mips/Fast-ISel/simplestorefp1.ll (added)
>>> +++ llvm/trunk/test/CodeGen/Mips/Fast-ISel/simplestorefp1.ll Sat 
>>> Jun  7 22:30:32 2014
>>> @@ -0,0 +1,39 @@
>>> +; RUN: llc -march=mipsel -relocation-model=pic -O0 -mips-fast-isel 
>>> -fast-isel-abort -mcpu=mips32r2 \
>>> +; RUN:     < %s | FileCheck %s
>>> +
>>> + at f = common global float 0.000000e+00, align 4
>>> + at de = common global double 0.000000e+00, align 8
>>> +
>>> +; Function Attrs: nounwind
>>> +define void @f1() #0 {
>>> +entry:
>>> +  store float 0x3FFA76C8C0000000, float* @f, align 4
>>> +  ret void
>>> +; CHECK:    .ent    f1
>>> +; CHECK:    lui    $[[REG1:[0-9]+]], 16339
>>> +; CHECK:    ori    $[[REG2:[0-9]+]], $[[REG1]], 46662
>>> +; CHECK:    mtc1    $[[REG2]], $f[[REG3:[0-9]+]]
>>> +; CHECK:    lw    $[[REG4:[0-9]+]], %got(f)(${{[0-9]+}})
>>> +; CHECK:    swc1    $f[[REG3]], 0($[[REG4]])
>>> +; CHECK:     .end    f1
>>> +
>>> +}
>>> +
>>> +; Function Attrs: nounwind
>>> +define void @d1() #0 {
>>> +entry:
>>> +  store double 1.234567e+00, double* @de, align 8
>>> +; CHECK:    .ent    d1
>>> +; CHECK:    lui    $[[REG1a:[0-9]+]], 16371
>>> +; CHECK:    ori    $[[REG2a:[0-9]+]], $[[REG1a]], 49353
>>> +; CHECK:    lui    $[[REG1b:[0-9]+]], 21403
>>> +; CHECK:    ori    $[[REG2b:[0-9]+]], $[[REG1b]], 34951
>>> +; CHECK:    mtc1    $[[REG2b]], $f[[REG3b:[0-9]+]]
>>> +; CHECK:    mtc1    $[[REG2a]], $f[[REG3a:[0-9]+]]
>>> +; CHECK:    sdc1    $f[[REG3b]], 0(${{[0-9]+}})
>>> +; CHECK:    .end    d1
>>> +  ret void
>>> +}
>>> +
>>> +attributes #0 = { nounwind "less-precise-fpmad"="false" 
>>> "no-frame-pointer-elim"="true" "no-frame-pointer-elim-non-leaf" 
>>> "no-infs-fp-math"="false" "no-nans-fp-math"="false" 
>>> "stack-protector-buffer-size"="8" "unsafe-fp-math"="false" 
>>> "use-soft-float"="false" }
>>> +
>>>
>>>
>>> _______________________________________________
>>> llvm-commits mailing list
>>> llvm-commits at cs.uiuc.edu
>>> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>>
>

-- 
http://www.nuanti.com
the browser experts




More information about the llvm-commits mailing list