[PATCH] D20907: [IfConversion] Bugfix: don't add Undef flag on use if reg is live.
Quentin Colombet via llvm-commits
llvm-commits at lists.llvm.org
Thu Jul 7 10:33:48 PDT 2016
> On Jul 6, 2016, at 3:30 PM, Matthias Braun <matze at braunis.de> wrote:
>
> MatzeB added a comment.
>
> In http://reviews.llvm.org/D20907#473455, @jonpa wrote:
>
>> Matthias, from what I can see you have some experience with this part of IfConversion.cpp. Perhaps you could give some advice on or even fix this problem? In short, a bug in IfConversion was found and fixed, but when patch was applied, we ran into a problem relating to Redefs - see above.
>
>
> To come back to this discussion: I added the UpdatePredRedefs() code to achieve the liveness effects stated in my previous post. I have no idea why I used the undef flag, it was one of my first commits to llvm so I assume I misunderstood the semantics back then. So removing this flag is the right call.
I believe Pete added that code, so you’re clear :P.
>
> To fix calls clobbering a live register; "pseudo" live in the sense that it is only live in our "ignore the predicates" worldview. I believe we have to add a implicit def and(!) and implicit use in the presence of clobbers or dead definitions.
The problem, as far as I understood, is that the liveness information is wrong when we call updatepredreg, which is why the add of implicit-use gets buggy.
>
>> I am now on vacation, and cannot not fix IfConversion right now, so if anybody could take the time that would be great, as this is a bug that should be fixed ASAP.
>
>
> While all of this is really nasty, it is hardly a new thing. This was broken before my commit in 2013 and as you found out it still is. I try to find time for this soon, but no promises.
I believe the “only” actual problem is the machine verifier (rightly) complaining about the liveness, but the new patch should not produce any miscompile unlike the previous state.
>
>
> http://reviews.llvm.org/D20907
>
>
>
More information about the llvm-commits
mailing list