[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