[PATCH] D129980: [RISCV] Optimize (seteq (i64 (and X, 0xffffffff)), C1)

Mon Jul 18 09:49:58 PDT 2022

craig.topper added a comment.

In D129980#3660144 <https://reviews.llvm.org/D129980#3660144>, @reames wrote:

> In D129980#3660107 <https://reviews.llvm.org/D129980#3660107>, @craig.topper wrote:
>
>> In D129980#3660043 <https://reviews.llvm.org/D129980#3660043>, @reames wrote:
>>
>>> LGTM
>>>
>>> Correct me if I'm wrong, but this should improve the idiomatic icmp eq i32 patterns for ELEN=64 right?  If so, have you thought about analogous cases for 16 and 8 bits w/Zb*?  On the surface, using a sext.h/b over a zext.h/b wouldn't seem to matter, but the constant simplifications are potentially interesting.  Should we be canonicalizing towards sext anyways?
>>
>> I don't think I understand this question. ELEN=64 is a vector term, but we're talking about scalar instructions here. Unless you meant XLEN=64?
>
> Oops, yes.

SelectionDAG type legalization is biased to prefer sign extend for all i32 compares via the `isSExtCheaperThanZExt` check in DAGTypeLegalizer::PromoteSetCCOperands. The cases I've seen this patch affect seem to come from the middle end with i64 type.

There is a generic fold to convert sext_in_reg to AND for equality compares(the opposite of this patch) in TargetLowering::SimplifySetCC. It is disabled for RISCV-V by `isSExtCheaperThanZExt`.

I think you're right there might be some be some interesting constant simplifications. I'm not sure it's restricted to Zb either.  i16 constants in the range [63488, 65535] could be sign extended to enable the use of `li` instead of `lui+addi`. All i8 constants for this should fit in `li` already, but I think sign extending could enable `c.li`. Not sure if it makes sense to always use sext for i8 and i16 type legalization via `isSExtCheaperThanZExt`.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D129980