[PATCH] D121319: Tests for D121320

[Sanjay Patel via Phabricator via llvm-commits]

spatel added a comment.

In D121319#3372983 <https://reviews.llvm.org/D121319#3372983>, @MatzeB wrote:

> In D121319#3372603 <https://reviews.llvm.org/D121319#3372603>, @spatel wrote:
>
>> I looked at the effect of the 2 proposed patches together, and we should add at least 2 more tests (can be added to this commit or separately):
>>
>>   define i1 @shifted_mask_testb_lower32(i64 %a) {
>>     %v0 = and i64 %a, 66846720  ; 0xff << 18
>>     %v1 = icmp ne i64 %v0, 0
>>     ret i1 %v1
>>   }
>
> This codegens as `testl $66846720, %edi ; setne %al` without any of our changes which to me seems to be as good as it gets. Using a shift for the "shifted_mask" cases only makes sense when the constant value becomes so big that it no longer fits the 32bit immediates on X86...
>
> I will add the function anyway as an example for something that should not be optimized, does that make sense?
>
>>   define i1 @shifted_mask_testw_lower32(i64 %a) {
>>     %v0 = and i64 %a, 131072 ; 0xffff << 1
>
> I guess this should have been `131070` :)

Oops - yes, I missed dropping the last bit.

>> The difference for these is that the shifted mask does not extend into the upper 32-bits of the value. We probably want to convert these to use shifts too, and we'll handle that with both patches applied.
>
> I think those should keep using `testl` + immediate.

I'm not sure if there's a universal answer. As you mentioned, it may depend on throughput vs. saving on instruction size. Either way, the tests can be there to show current/expected codegen.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D121319