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

Eric Christopher echristo at gmail.com
Wed Feb 19 17:06:30 PST 2014


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

>
> 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
>
>



More information about the llvm-commits mailing list