[PATCH] D111497: m68k: Support bit shifts on 64-bit integers

[Jessica Clarke via Phabricator via llvm-commits]

jrtc27 added a comment.

In D111497#3079635 <https://reviews.llvm.org/D111497#3079635>, @ricky26 wrote:

> @AnnikaCodes, I'm seeing test failures from this patch. It looks like it's generating SETCC nodes which don't return i8 (which the M68k requires).
>
>   FAIL: LLVM :: CodeGen/M68k/Arith/bitwise.ll (315 of 45926)
>   ******************** TEST 'LLVM :: CodeGen/M68k/Arith/bitwise.ll' FAILED ********************
>   Script:
>   --
>   : 'RUN: at line 2';   /home/ricky26/Projects/OSS/m68k/llvm/build-relwithdebinfo/bin/llc < /home/ricky26/Projects/OSS/m68k/llvm/llvm/test/CodeGen/M68k/Arith/bitwise.ll -mtriple=m68k-linux -verify-machineinstrs | /home/ricky26/Projects/OSS/m68k/llvm/build-relwithdebinfo/bin/FileCheck /home/ricky26/Projects/OSS/m68k/llvm/llvm/test/CodeGen/M68k/Arith/bitwise.ll
>   --
>   Exit Code: 1
>   
>   Command Output (stderr):
>   --
>   llc: /home/ricky26/Projects/OSS/m68k/llvm/llvm/lib/Target/M68k/M68kISelLowering.cpp:1914: llvm::SDValue llvm::M68kTargetLowering::LowerSETCC(llvm::SDValue, llvm::SelectionDAG&) const: Assertion `VT == MVT::i8 && "SetCC type must be 8-bit integer"' failed.
>   PLEASE submit a bug report to https://bugs.llvm.org/ and include the crash backtrace.
>   Stack dump:
>   0.	Program arguments: /home/ricky26/Projects/OSS/m68k/llvm/build-relwithdebinfo/bin/llc -mtriple=m68k-linux -verify-machineinstrs
>   1.	Running pass 'Function Pass Manager' on module '<stdin>'.
>   2.	Running pass 'M68k DAG->DAG Pattern Instruction Selection' on function '@lshr64'
>   -snip-
>    #8 0x0000557b6ce29aee llvm::M68kTargetLowering::LowerSETCC(llvm::SDValue, llvm::SelectionDAG&) const /home/ricky26/Projects/OSS/m68k/llvm/llvm/lib/Target/M68k/M68kISelLowering.cpp:1976:1
>    #9 0x0000557b6ce2a251 llvm::M68kTargetLowering::LowerSELECT(llvm::SDValue, llvm::SelectionDAG&) const /home/ricky26/Projects/OSS/m68k/llvm/llvm/lib/Target/M68k/M68kISelLowering.cpp:2039:5
>   #10 0x0000557b6dde95a0 (anonymous namespace)::SelectionDAGLegalize::LegalizeOp(llvm::SDNode*) /home/ricky26/Projects/OSS/m68k/llvm/llvm/lib/CodeGen/SelectionDAG/LegalizeDAG.cpp:1281:43
>
> I checked the debug output:
>
>   Legalizing: t24: i32,i32 = srl_parts t5, t3, t9
>   Trying custom legalization
>   Creating constant: t25: i32 = Constant<0>
>   Creating constant: t26: i32 = Constant<1>
>   Creating constant: t27: i32 = Constant<-32>
>   Creating constant: t28: i32 = Constant<31>
>   Creating new node: t29: i32 = add t9, Constant:i32<-32>
>   Creating new node: t30: i32 = sub Constant:i32<31>, t9
>   Creating new node: t31: i32 = srl t5, t9
>   Creating new node: t32: i32 = shl t3, Constant:i32<1>
>   Creating new node: t33: i32 = shl t32, t30
>   Creating new node: t34: i32 = or t31, t33
>   Creating new node: t35: i32 = srl t3, t9
>   Creating new node: t36: i32 = srl t3, t29
>   Creating new node: t38: i32 = setcc t29, Constant:i32<0>, setlt:ch
>   Creating new node: t39: i32 = select t38, t34, t36
>   Creating new node: t40: i32 = select t38, t35, Constant:i32<0>
>   Creating new node: t41: i32,i32 = merge_values t39, t40
>   Successfully custom legalized node
>    ... replacing: t24: i32,i32 = srl_parts t5, t3, t9
>        with:      t41: i32,i32 = merge_values t39, t40
>         and:      t41: i32,i32 = merge_values t39, t40
>
> If I understand correctly, I think the only issue is that `t38` returns `i32` instead of `i8`.
>
> It might be as simple as making it so that the `getSetCC` calls use `MVT::i8` instead of the input `VT`.

That sounds right. On RISC-V, XLenVT is the only legal integer type, and thus what is used by getSetCCResultType (ignoring vectorisation), whereas M68k's returns i8, something I missed when outlining how to generalise the RISC-V implementation.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D111497