[PATCH] D142254: [X86] Transform vector SET{LE/ULT/ULE} -> SETLT and SET{GE/UGT/UGE} -> SETGT if possible

[Sterling Augustine via Phabricator via llvm-commits]

saugustine accepted this revision.
saugustine added a comment.

The fix does work. But I'm still trying to get a reproducer for the future.

Incidentally, the function comment explicitly says "or this is not a simple vector constant", and yet originally didn't check for it actually being simple. With the updated fix it does.

So I think this is correct.

In D142254#4130495 <https://reviews.llvm.org/D142254#4130495>, @goldstein.w.n wrote:

> In D142254#4130325 <https://reviews.llvm.org/D142254#4130325>, @saugustine wrote:
>
>> I think it probably needs to also check if the type is Simple, because the exact assertion that fails is isSimple.
>>
>> And if I insert a check for isSimple on line 24731, the test does pass.
>>
>>   if (!BV || !V.getValueType().isSimple())
>>
>> Not sure if that is the correct fix, but perhaps better than reverting. The test case is huge and buried.
>
> Are you going to be able to confirm that fix works? Or is it too complicated to reproduce?




Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D142254