Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Cannot select SystemZISD::GET_CCMASK #59054

Closed
JonPsson opened this issue Nov 17, 2022 · 2 comments
Closed

Cannot select SystemZISD::GET_CCMASK #59054

JonPsson opened this issue Nov 17, 2022 · 2 comments
Assignees

Comments

@JonPsson
Copy link
Contributor

(Bisected and seems to be present at least one year back)

testcase.tar.gz
llc ./tc_GET_CCMASK.ll
LLVM ERROR: Cannot select: t50: i32 = SystemZISD::GET_CCMASK t72, Constant:i32<15>, Constant:i32<3>

Legalized selection DAG: %bb.0 'k:bb'
SelectionDAG has 35 nodes:
    t61: i64,i32 = SystemZISD::UADDO undef:i64, undef:i64
  t62: i32 = SystemZISD::SELECT_CCMASK Constant:i32<1>, Constant:i32<0>, TargetConstant:i32<15>, TargetConstant:i32<3>, t61:1
                  t50: i32 = SystemZISD::GET_CCMASK t62, Constant:i32<15>, Constant:i32<3>

=>

Optimized legalized selection DAG: %bb.0 'k:bb'
SelectionDAG has 30 nodes:
  t61: i64,i32 = SystemZISD::UADDO undef:i64, undef:i64
  t71: i64 = SystemZISD::SELECT_CCMASK Constant:i64<1>, Constant:i64<0>, TargetConstant:i32<15>, TargetConstant:i32<3>, t61:1
  t72: i32 = truncate t71
                 t50: i32 = SystemZISD::GET_CCMASK t72, Constant:i32<15>, Constant:i32<3>
SystemZTargetLowering::combineGET_CCMASK():
  if (Select->getOpcode() != SystemZISD::SELECT_CCMASK)
=>  return SDValue();

It seems to me that the inserted truncate node is not handled - maybe it should either never be emitted or it should be recognized in combineGET_CCMASK().

@uweigand
Copy link
Member

This starts out as

  t20: i64,i32 = uaddo undef:i64, undef:i64
    t0: ch = EntryToken
              t6: i64 = select t20:1, Constant:i64<9223372036854775807>, Constant:i64<-1>
            t37: i64,i32 = addcarry t6, Constant:i64<0>, t20:1
              t38: i64 = any_extend t20:1
            t23: i64 = sign_extend_inreg t38, ValueType:ch:i1
          t8: i64 = and t37, t23

which is a bit special in that the carry output of the uaddo is used multiple times.

After legalization we have:

    t61: i64,i32 = SystemZISD::UADDO undef:i64, undef:i64
  t62: i32 = SystemZISD::SELECT_CCMASK Constant:i32<1>, Constant:i32<0>, TargetConstant:i32<15>, TargetConstant:i32<3>, t61:1
    t0: ch = EntryToken
                    t65: i32 = SystemZISD::ICMP t62, Constant:i32<0>, TargetConstant:i32<0>
                  t67: i64 = SystemZISD::SELECT_CCMASK Constant:i64<9223372036854775807>, Constant:i64<-1>, TargetConstant:i32<14>, TargetConstant:i32<6>, t65
                  t50: i32 = SystemZISD::GET_CCMASK t62, Constant:i32<15>, Constant:i32<3>
                t51: i64,i32 = SystemZISD::ADDCARRY t67, Constant:i64<0>, t50
                    t38: i64 = any_extend t62
                  t57: i64 = and t38, Constant:i64<1>

Now several platform optimizations apply. The result of the first SELECT_CCMASK (t62) is used as input to any ANY_EXTEND, which presumably gets modified to a ZERO_EXTEND by common code somewhere, which causes SystemZTargetLowering::combineZERO_EXTEND to trigger and fold the extend into the SELECT_CCMASK. As there are other users of the original t62 value, this also creates the TRUNCATE - which now prevents the GET_CCMASK to be recognized.

I think there are two options to fix this. Either, combineZERO_EXTEND could scan the list of uses of the original SELECT_CCMASK, and if one of them is a GET_CCMASK, drop the optimization. Or else, combineGET_CCMASK could accept intermediate TRUNCATE nodes - this is valid if we can prove the truncation doesn't affect the outcome of a test against zero, i.e. we can prove the TRUNCATE node is zero (in the smaller mode) if and only if the original node is zero (in the wider mode). This will always be true for those TRUNCATE nodes that were added by the combineZERO_EXTEND optimization.

@JonPsson
Copy link
Contributor Author

Aha - so the truncate is actually also produced by the SystemZ backend.

I thought it might be a little better to keep the handling of GET_CCMASK inside combineGET_CCMASK(), so I chose to handle the truncate there. IIUC, the operands will always be constant 0 or 1, which are the values I check for, which should always be safe to truncate. Proposed patch: https://reviews.llvm.org/D138324

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants