[PATCH] D57243: GlobalISel: Allow bitcount ops to have different result type

Roman Tereshin via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Tue Feb 5 11:03:55 PST 2019


rtereshin added a comment.

In D57243#1385511 <https://reviews.llvm.org/D57243#1385511>, @qcolombet wrote:

> Should we revert this change to have the conversation going or am I the only one unhappy with the absence of guarantees for the other operands?


I'm not happy with the current state too!

Though, maybe reverting is a bit of a hassle, there is already a follow-up committed later that fixes functional bugs in here (https://reviews.llvm.org/rL353102), it'll have to be reverted too. Maybe a follow up commit instead?

In general, I think we can continue figuring out what the contract needs to be, and adopt "don't change type indices that aren't explicitly requested to be changed unless it's impossible to do so w/o introducing broken MIR" as a guideline right away.

Also, it's just a gut feeling, so it could be a complete nonsense as I didn't give it much thought, but I feel especially bad about the version that widens one type index and narrows the other one. At least just widening more than asked to strictly increases the entire type tuple (as in partial order). Skewing the type tuple like so on the other hand might be more loop-prone, I think.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D57243/new/

https://reviews.llvm.org/D57243





More information about the llvm-commits mailing list