[PATCH] D121319: Tests for D121320

Matthias Braun via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Thu Mar 10 09:14:26 PST 2022


MatzeB added a comment.

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` :)

>   %v1 = icmp ne i64 %v0, 0
>   ret i1 %v1
>
> }
>
>   

This ends up being just another variant of the other function where it is best to just use a `testl` with immediate, so I think I can skip this.

> 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.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D121319



More information about the llvm-commits mailing list