[PATCH] D29511: [X86]: Don't set a regmask on conditional tail calls (PR31257)

[Hans Wennborg via Phabricator via llvm-commits]

hans added a comment.

In https://reviews.llvm.org/D29511#668161, @mkuper wrote:

> Sorry for the lag, Hans.
>
> I thought about this a bit more, and I'm not sure I'm a fan of this, even though I basically suggested something similar on Friday.
>
> Two issues:
>
> 1. I'm still not sure this won't have any unintended consequences.
> 2. Having BuildMI return an instruction that requires you to remove an operand for correctness seems rather inelegant. I agree that this is probably "safer" than removing IsCall, and that is probably the only code that will ever construct a TCRETURNdi64cc MI, but, still...
>
>   I don't have any good ideas, though.


I agree with 2), that code is clumsy at best. I think what happened is that we regressed this in r281223: we shouldn't have copied the regmask in the first place. We don't have to remove an operand for correctness, it was incorrect to add it. I'm uploading a new patch that does this.

As for constructing TCRETURNdi64cc elsewhere, well they shouldn't put regmask on it either.

For 1) well, that's the case with a lot of code. I'm feeling pretty good about this change though. (I don't think removing isCall would actually work, as the regmask is copied over from the unconditional tail call. I suppose we could remove isCall from that, but that doesn't feel right at all.)


https://reviews.llvm.org/D29511