[llvm] r210414 - Do materialize for floating point

Reed Kotler reed.kotler at imgtec.com
Sun Jun 8 02:51:27 PDT 2014


You should not have reverted this patch.

For one, you are not an authorized llvm Mips mantainer; and I am.
This is Mips target specific code.

Phabricator is not a required part of the check in process for llvm.
Daniel and I use it as part of internal pre-commit review that we do at 
Mips/Imagination.
It's not meant for public review.

I am entitled to check in code without pre-commit review.



Reed

On 06/08/2014 02:40 AM, Alp Toker wrote:
>
> 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
>>>
>>
>




More information about the llvm-commits mailing list