[PATCH] ScheduleDAGInstrs should toggle kill flags on bundled instrs

[Andrew Trick]

> On May 1, 2015, at 5:48 PM, Pete Cooper <peter_cooper at apple.com> wrote:
> 
> Hi Andy
> 
> The attached patch (and test case) is a bug found by the machine verifier.  The problem is that prior to scheduling we had
> 
> %R3<def> = t2ANDrr %R0 …
> 	BUNDLE %ITSTATE<imp-def,dead>, %R6<imp-def>, %R0<imp-use,kill>, %R4<imp-use,kill>, %CPSR<imp-use,kill>, %R6<imp-use,kill>
> 	  * t2IT 1, 24, %ITSTATE<imp-def>
> 	  * %R6<def,tied6> = t2ORRrr %R0<kill>, %R4<kill>, pred:1, pred:%CPSR<kill>, opt:%noreg, %R6<imp-use,kill,tied0>, %ITSTATE<imp-use,kill,internal>
> 
> but after scheduling we moved the AND later, and cleared the kill flag only from the imp-use on the BUNDLE, not the kill on any of the instructions inside the bundle.
> 
> The verifier then thinks that this bundle killed R0, and so the AND after it is reading an undefined physreg.
> 
> This patch updates toggleKillFlag to also toggle the kill flags of instructions inside the bundle to match.  It does this bottom up in the bundle as if we are setting a kill flag then we only want to set it on the last use.
> 
> The test here demonstrates the issue, although i don’t think its suitable for commit.  Constructing a test which will have these bundled instructions along with guaranteeing that the AND will be scheduled later just isn’t feasible.  I hope this is ok.
> 
> Let me know what you think.

I don’t have a problem with the fix. I do have a problem with the fact that a backend is still calling finalizeBundles. It’s an invitation for bugs. I thought we had successfully obsoleted the BUNDLE instruction with MIBundleOperands. Are there ARM specific passes that just need to be converted to MIBundleOperands.

Anyway, I don’t want to hold this fix up, so LGTM, with a plea for fixing bundles the right way..

Andy

> 
> Cheers
> Pete
> 
> <scheduler-bundle-kills.diff><opt.ll>