[llvm] r253664 - [Hexagon] Remove redundant assignment.

[Krzysztof Parzyszek via llvm-commits]

Hi,
The code there is buggy.  The intent was to return "true" if the pass 
made *any* change to the IR (regardless of whether that was because of 
deleting dead code, or because new instructions were added).  The code, 
as written, does not do that properly.  The dead code elimination was 
added later on as we discovered problems with the original approach 
(calling the CodeGen's implementation), and the patches did not 
correctly updated the return value.

I'll fix it.  Thanks for pointing this out.

-Krzysztof



On 11/20/2015 2:00 PM, Tilmann Scheller wrote:
> Hi David,
>
> double checked this one: runOnMachineFunction() has two early exits
> returning false plus the return of Changed at the very end of the
> function. The return at the end of the function is the only use of Changed.
>
> “Changed = “ is correct as generateInserts() always returns true.
>
> Technically “Changed” can be optimized away completely (returning true
> at the end of runOnMachineFunction()). That would probably the best
> solution to make the current behavior explicit.
>
> Adding Krzysztof (the original author of the file) to get his input.
>
> Regards,
>
> Tilmann
>
> *From:*David Blaikie [mailto:dblaikie at gmail.com]
> *Sent:* Friday, November 20, 2015 7:19 PM
> *To:* Tilmann Scheller
> *Cc:* llvm-commits
> *Subject:* Re: [llvm] r253664 - [Hexagon] Remove redundant assignment.
>
> Not sure if that's the right fix - it's posisble that the later use of
> "Changed" should've been "Changed |=" instead of "Changed =" - again,
> probably best to address this by replying to the original commit, rather
> than just making the change outright like this.
>
> On Fri, Nov 20, 2015 at 5:27 AM, Tilmann Scheller via llvm-commits
> <llvm-commits at lists.llvm.org <mailto:llvm-commits at lists.llvm.org>> wrote:
>
> Author: tilmann
> Date: Fri Nov 20 07:27:30 2015
> New Revision: 253664
>
> URL: http://llvm.org/viewvc/llvm-project?rev=253664&view=rev
> Log:
> [Hexagon] Remove redundant assignment.
>
> Identified by the Clang static analyzer.
>
> Modified:
>      llvm/trunk/lib/Target/Hexagon/HexagonGenInsert.cpp
>
> Modified: llvm/trunk/lib/Target/Hexagon/HexagonGenInsert.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/Hexagon/HexagonGenInsert.cpp?rev=253664&r1=253663&r2=253664&view=diff
> ==============================================================================
> --- llvm/trunk/lib/Target/Hexagon/HexagonGenInsert.cpp (original)
> +++ llvm/trunk/lib/Target/Hexagon/HexagonGenInsert.cpp Fri Nov 20
> 07:27:30 2015
> @@ -1495,7 +1495,7 @@ bool HexagonGenInsert::runOnMachineFunct
>     // version of DCE that preserves lifetime markers. Without it, merging
>     // of stack objects can fail to recognize and merge disjoint objects
>     // leading to unnecessary stack growth.
> -  Changed |= removeDeadCode(MDT->getRootNode());
> +  removeDeadCode(MDT->getRootNode());
>
>     const HexagonEvaluator HE(*HRI, *MRI, *HII, MF);
>     BitTracker BTLoc(HE, MF);
>
>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at lists.llvm.org <mailto:llvm-commits at lists.llvm.org>
> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
>


-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, 
hosted by The Linux Foundation