[PATCH] D89390: [TargetLowering] Update optimization to check for boolean content

[Erik Hogeman via Phabricator via llvm-commits]

ErikHogeman added a comment.

I took at look, and there is one thing I thought I should clarify with you before I push another diff. So the code looks like this before applying this patch:

  ld.param.u32    %r1, [pow2_mask_cmp_param_0];
  bfe.u32     %r2, %r1, 3, 1;
  st.param.b32    [func_retval0+0], %r2;
  ret;

After this patch is applied:

  ld.param.u32    %r1, [pow2_mask_cmp_param_0];
  and.b32     %r2, %r1, 8;
  setp.ne.s32     %p1, %r2, 0;
  selp.u32    %r3, 1, 0, %p1;
  st.param.b32    [func_retval0+0], %r3;
  ret;

So it looks like we generate more instructions when disabling this transformation, which is unfortunate.
But I'm thinking now, this transformation is probably correct for setcc nodes with i1 type regardless of the boolean content, so maybe I should update the patch to only check the boolean contents when the type has a larger bitsize than 1?
That would make sure we still generate the code as in the first case above, but it would mean I'm back to not being sure how to write a good test for this.
Any thoughts?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D89390