[llvm] r201723 - Make one statement easier to understand from post commmit feedback from a

[Eric Christopher]

On Wed, Feb 19, 2014 at 5:16 PM, reed kotler <rkotler at mips.com> wrote:
> On 02/19/2014 05:06 PM, Eric Christopher wrote:
>>
>> On Wed, Feb 19, 2014 at 5:01 PM, reed kotler <rkotler at mips.com> wrote:
>>>
>>> On 02/19/2014 04:44 PM, Eric Christopher wrote:
>>>
>>> The comment should be fine, you don't need to leave the commented out
>>> code in there.
>>>
>>> -eric
>>>
>>>   I want that code put in there when the issue is fixed in
>>> MipsAsmPrinter.
>>> If I delete it then I, or someone else, needs to re-figure out the right
>>> incantation.
>>
>> Doesn't look particularly hard. Standard policy is that commented out
>> code should be deleted. If it's hard to resurrect then you can comment
>> out what the conditional should be.
>>
>> -eric
>
> That is what I did. I commented out what the conditional should be.

Prose, not code.

-eric

>
> Reed
>
>>> If the stub does not need help returning then it does not need to save
>>> S2.
>>> But to support that, Asm printer needs to generate a different kind of
>>> call.
>>>
>>> If I used raw text I would just generate it but that is not allowed
>>> anymore
>>> so I need to create
>>> MipsMCExpr and add some special methods to do that. I did not want to
>>> make
>>> an already
>>> large patch larger by adding it.
>>>
>>> Reed
>>>
>>>
>>>
>>>
>>> On Wed, Feb 19, 2014 at 2:59 PM, reed kotler <rkotler at mips.com> wrote:
>>>
>>> Here is a proposed patch then:
>>>
>>> rkotler at mipssw006:~/llvm_trunk$ svn diff
>>> Index: lib/Target/Mips/Mips16ISelLowering.cpp
>>> ===================================================================
>>> --- lib/Target/Mips/Mips16ISelLowering.cpp    (revision 201723)
>>> +++ lib/Target/Mips/Mips16ISelLowering.cpp    (working copy)
>>> @@ -467,8 +467,11 @@
>>>
>>>             // So for now we always save S2. The optimization will be
>>> done
>>>             // in a follow-on patch.
>>>             //
>>> -          if (1 || (Signature->RetSig != Mips16HardFloatInfo::NoFPRet))
>>> -            FuncInfo->setSaveS2();
>>> +          // FIXME: add this conditional when optimization to not save
>>> S2
>>> +          // can be satisfied by AsmPrinter.
>>> +          //
>>> +          // if (Signature->RetSig != Mips16HardFloatInfo::NoFPRet)
>>> +          FuncInfo->setSaveS2();
>>>
>>>           }
>>>           // one more look at list of intrinsics
>>>           if (std::binary_search(Mips16IntrinsicHelper,
>>>
>>>
>>>
>>> On 02/19/2014 02:35 PM, Eric Christopher wrote:
>>>
>>> Uh, this is just if (true)... how about just put a FIXME in there and
>>> remove the conditional?
>>>
>>> -eric
>>>
>>> On Wed, Feb 19, 2014 at 2:11 PM, Reed Kotler <rkotler at mips.com> wrote:
>>>
>>> Author: rkotler
>>> Date: Wed Feb 19 16:11:45 2014
>>> New Revision: 201723
>>>
>>> URL: http://llvm.org/viewvc/llvm-project?rev=201723&view=rev
>>> Log:
>>> Make one statement easier to understand from post commmit feedback from a
>>> review of the previous patch that introduced this week.
>>>
>>>
>>> Modified:
>>>       llvm/trunk/lib/Target/Mips/Mips16ISelLowering.cpp
>>>
>>> Modified: llvm/trunk/lib/Target/Mips/Mips16ISelLowering.cpp
>>> URL:
>>>
>>> http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/Mips/Mips16ISelLowering.cpp?rev=201723&r1=201722&r2=201723&view=diff
>>>
>>>
>>> ==============================================================================
>>> --- llvm/trunk/lib/Target/Mips/Mips16ISelLowering.cpp (original)
>>> +++ llvm/trunk/lib/Target/Mips/Mips16ISelLowering.cpp Wed Feb 19 16:11:45
>>> 2014
>>> @@ -467,7 +467,7 @@ getOpndList(SmallVectorImpl<SDValue> &Op
>>>              // So for now we always save S2. The optimization will be
>>> done
>>>              // in a follow-on patch.
>>>              //
>>> -          if (Signature->RetSig != Mips16HardFloatInfo::NoFPRet || 1)
>>> +          if (1 || (Signature->RetSig != Mips16HardFloatInfo::NoFPRet))
>>>                FuncInfo->setSaveS2();
>>>            }
>>>            // one more look at list of intrinsics
>>>
>>>
>>> _______________________________________________
>>> llvm-commits mailing list
>>> llvm-commits at cs.uiuc.edu
>>> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>>>
>>>
>