[PATCH] D29511: [X86]: Don't set a regmask on conditional tail calls (PR31257)
Hans Wennborg via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Mon Feb 6 15:32:05 PST 2017
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
More information about the llvm-commits
mailing list