Quuxplusone / LLVMBugzillaTest

0 stars 0 forks source link

m68k: Can't use bitshift operators (>> and <<) on 64-bit integers #51086

Open Quuxplusone opened 3 years ago

Quuxplusone commented 3 years ago
Bugzilla Link PR52119
Status CONFIRMED
Importance P normal
Reported by Annika (annika0uwu@gmail.com)
Reported on 2021-10-08 18:50:19 -0700
Last modified on 2021-11-02 23:47:18 -0700
Version trunk
Hardware All All
CC annika0uwu@gmail.com, glaubitz@physik.fu-berlin.de, id@timryan.org, jrtc27@jrtc27.com, llvm-bugs@lists.llvm.org, minyihh@uci.edu
Fixed by commit(s)
Attachments llc_2021-10-08-184648_annika-mac.crash (11825 bytes, text/plain)
0001-patch-Applies-patch-for-52119-m68k-Can-t-use-bitshif.patch (1055 bytes, text/plain)
Blocks
Blocked by
See also

Created attachment 25344 Diagnostic report when attempting to compile the generated LLVM-IR with llc.

LLVM's new m68k target can't handle left-shifts and right-shifts of two 64-bit integers. This bug isn't specific to C or Clang (it can be reproduced with Rust as well), but the following C program is an easy way to reproduce it:

void llvm_bug() { unsigned long long x = 2; unsigned long long y = 2; unsigned long long result = x >> y; }

It builds fine under an m68k GCC cross-compiler, but under Clang, throws the following error: Cannot select: 0x7fc3c1017300: i32,i32 = srl_parts 0x7fc3c10171c8, 0x7fc3c1017160, 0x7fc3c1017090.

This is the output when building the above program (saved as shift.c) with bin/clang -target m68k-unknown-linux-gnu shift.c -nostdlib: fatal error: error in backend: Cannot select: 0x7f9644092b00: i32,i32 = srl_parts 0x7f96440929c8, 0x7f9644092960, 0x7f9644092890 0x7f96440929c8: i32,ch = load<(dereferenceable load (s32) from %ir.1 + 4, basealign 8)> 0x7f96440927c0, 0x7f96440926f0, undef:i32 0x7f96440926f0: i32 = or FrameIndex:i32<0>, Constant:i32<4> 0x7f9644091e68: i32 = FrameIndex<0> 0x7f96440924e8: i32 = Constant<4> 0x7f9644091f38: i32 = undef 0x7f9644092960: i32,ch = load<(dereferenceable load (s32) from %ir.1, align 8)> 0x7f96440927c0, FrameIndex:i32<0>, undef:i32 0x7f9644091e68: i32 = FrameIndex<0> 0x7f9644091f38: i32 = undef 0x7f9644092890: i32,ch = load<(dereferenceable load (s32) from %ir.2 + 4, basealign 8)> 0x7f96440927c0, 0x7f96440928f8, undef:i32 0x7f96440928f8: i32 = or FrameIndex:i32<1>, Constant:i32<4> 0x7f9644092008: i32 = FrameIndex<1> 0x7f96440924e8: i32 = Constant<4> 0x7f9644091f38: i32 = undef In function: llvm_bug clang-14: error: clang frontend command failed with exit code 70 (use -v to see invocation) clang version 14.0.0 (https://github.com/llvm/llvm-project.git 9697f93587f46300814f1c6c68af347441d6e05d) Target: m68k-unknown-linux-gnu Thread model: posix InstalledDir: /Users/annika/llvm-project/build/bin clang-14: note: diagnostic msg:


PLEASE ATTACH THE FOLLOWING FILES TO THE BUG REPORT: Preprocessed source(s) and associated run script(s) are located at: clang-14: note: diagnostic msg: /var/folders/cn/qwypwgqn3vn7wwn5ksn9lr5w0000gn/T/shift-5ddeee.c clang-14: note: diagnostic msg: /var/folders/cn/qwypwgqn3vn7wwn5ksn9lr5w0000gn/T/shift-5ddeee.sh clang-14: note: diagnostic msg: Crash backtrace is located in clang-14: note: diagnostic msg: /Users/annika/Library/Logs/DiagnosticReports/clang-14.crash clang-14: note: diagnostic msg: (choose the .crash file that corresponds to your crash) clang-14: note: diagnostic msg:


For some reason, clang didn't generate the .crash file it was talking about, but compiling to LLVM-IR and then using llc made the proper diagnostic report, which I have attached.

I understand that m68k support is experimental, but I would still like to report this bug in hopes that it will either eventually be fixed, or someone can help me to fix it myself.

Thanks!

Quuxplusone commented 3 years ago

Attached llc_2021-10-08-184648_annika-mac.crash (11825 bytes, text/plain): Diagnostic report when attempting to compile the generated LLVM-IR with llc.

Quuxplusone commented 3 years ago
Known bug, was reported the other week on IRC, backend just needs to
setOperandAction(ISD::S{HL,RA,RL}_PARTS, MVT::i32, Expand). I thought someone
had posted that patch but can't find it so perhaps not. Test case is simply:

define i64 @lshr64(i64 %a, i64 %b) nounwind {
  %1 = lshr i64 %a, %b
  ret i64 %1
}

define i64 @ashr64(i64 %a, i64 %b) nounwind {
  %1 = ashr i64 %a, %b
  ret i64 %1
}

define i64 @shl64(i64 %a, i64 %b) nounwind {
  %1 = shl i64 %a, %b
  ret i64 %1
}
Quuxplusone commented 3 years ago
(In reply to Jessica Clarke from comment #1)
> Known bug, was reported the other week on IRC, backend just needs to
> setOperandAction(ISD::S{HL,RA,RL}_PARTS, MVT::i32, Expand). I thought
> someone had posted that patch but can't find it so perhaps not. Test case is
> simply:
>
> define i64 @lshr64(i64 %a, i64 %b) nounwind {
>   %1 = lshr i64 %a, %b
>   ret i64 %1
> }
>
> define i64 @ashr64(i64 %a, i64 %b) nounwind {
>   %1 = ashr i64 %a, %b
>   ret i64 %1
> }
>
> define i64 @shl64(i64 %a, i64 %b) nounwind {
>   %1 = shl i64 %a, %b
>   ret i64 %1
> }

Alright, thanks for the info! Should I submit a patch for this?
Quuxplusone commented 3 years ago
(In reply to Annika from comment #2)
> Alright, thanks for the info! Should I submit a patch for this?

If you have a patch fixing the issue, that would be much appreciated. Thank you!
Quuxplusone commented 3 years ago

Attached 0001-patch-Applies-patch-for-52119-m68k-Can-t-use-bitshif.patch (1055 bytes, text/plain): Implement SHL_PARTS, SRA_PARTS, and SRL_PARTS for i32

Quuxplusone commented 3 years ago
Hi Tim!

(In reply to Tim Ryan from comment #4)
> Hi, I was interested in fixing this upstream issue in LLVM M68K backend.
> Attached is a patch that applies the above recommendation (calling
> setOperationAction) for the three unimplemented operation types. Let me know
> if I can refine the patch, thanks.

There is an ongoing discussion where a patch for this is being discussed:

> https://reviews.llvm.org/D111497

Maybe you could join the discussion and help as the current patch introduces a
regression.

Thanks!