[PATCH] Teach IfConversion about register mask kills

[Pete Cooper]

Hi Matthias

Thanks for the feedback.  How about these 2 patches?  First is the refactoring you suggested.  Second is just the fix I gave earlier but in terms of the new StepForward API.

Cheers,
Pete

-------------- next part --------------
A non-text attachment was scrubbed...
Name: refactor.diff
Type: application/octet-stream
Size: 6368 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20150504/56a48b4b/attachment.obj>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: ifcvt.diff
Type: application/octet-stream
Size: 3313 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20150504/56a48b4b/attachment-0001.obj>
-------------- next part --------------


> On May 1, 2015, at 5:47 PM, Matthias Braun <mbraun at apple.com> wrote:
> 
> Hi Pete,
> 
> the patch itself looks ok to me, but now I feel bad about introducing that code duplication back then. I guess you could alternative change stepForward() to take an additional "SmallVector<unsigned,4> &" argument which gets the set of newly defined/regmask killed filled in. The function is really sparsely used anyway so no need to worry about the last bits of performance here.
> 
> - Matthias
> 
>> On May 1, 2015, at 4:14 PM, Pete Cooper <peter_cooper at apple.com> wrote:
>> 
>> Hi Quentin/Andy
>> 
>> When the if converter predicates an instruction, UpdatePredRedefs can add implicit uses to an instruction.  However, that code was not aware of register masks, unlike StepForward() which it references in its header.
>> 
>> The problem case we have here is something like
>> 
>> R0 = …
>> if fails stack check, jump to fail, else end
>> 
>> fail:
>> call ___stack_chk_fail <regmask clobbers R0>
>> // no return here, so no successors
>> 
>> end:
>> call free(R0)
>> 
>> Because the stack_chk_fail call doesn’t return, the live range for the R0 def wasn’t live across the ‘fail’ BB, so it was safe to use R0 here, despite the clobber.  After we flatten however, we have something like
>> 
>> R0 = …
>> call ___stack_chk_fail <regmask clobbers R0>
>> call free(R0)
>> 
>> so now the use of R0 fails the verifier because R0 was clobbered by the predicated call.
>> 
>> I discussed this with Quentin offline and we agreed that removing R0 from the regmask is definitely not a possibility as later code such as the post-RA scheduler could depend on it for correctness.
>> 
>> Quentin suggested instead that we add imp uses/defs to the call to show that we know a register was live across it, and we’re ok with it.  This then passes the verifier.
>> 
>> Cheers
>> Pete
>> 
>> <regmask-ifcvt.diff>_______________________________________________
>> llvm-commits mailing list
>> llvm-commits at cs.uiuc.edu
>> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>