riscvarchive / riscv-gcc

GNU General Public License v2.0
358 stars 273 forks source link

Compiler error on shifting 64-bits integer in RV32 #236

Open yetingk opened 3 years ago

yetingk commented 3 years ago
__attribute__((noipa))
void test(long long expr, long long *llp) {
  *llp = expr >> *llp;
}

int main(void) {
  long long lgot = 1;
  test(1LL << 32, &lgot);
  if (lgot != 1LL << 31)
    abort();
  return 0;
}

The code has rv32ima compiler error with -O1. I expect lgot to be 1LL << 31 after test(), but the vaule is 0x180000000. After cfgexpand pass, there are some incorrect memory references before "insn 43" (shift high-part). "insn 41" stores the low-part result in memory before "insn 42" loads the original low-part for "insn 43".

(insn 40 39 41 (set (reg:SI 100) (ior:SI (reg:SI 99) (reg:SI 97))) "test.c":5:15 -1 (nil))

(insn 41 40 42 (set (mem:SI (reg/v/f:SI 76 [ llp ]) [1 *llp_5(D)+0 S4 A64]) (reg:SI 100)) "test.c":5:15 -1 (nil))

(insn 42 41 43 (set (reg:SI 101) (mem:SI (reg/v/f:SI 76 [ llp ]) [1 *llp_5(D)+0 S4 A64])) "test.c":5:15 -1 (nil))

(insn 43 42 44 (set (reg:SI 102) (ashiftrt:SI (subreg:SI (reg/v:DI 75 [ expr ]) 4) (subreg:QI (reg:SI 101) 0))) "test.c":5:15 -1 (nil))

Also I found that the program could run correctly with fno-tree-ter. My gcc version is 10.1.0.

jim-wilson commented 3 years ago

I can reproduce with 10.2. I get the correct result with FSF gcc mainline. So it appears that the bug has already been fixed upstream. I don't think that this is a RISC-V bug, so technically it isn't our problem, though it would be nice to have a working compiler. Someone would have to do a bisection on the FSF GCC tree to find the patch that fixed it.

shaochung commented 3 years ago

The bug was fixed by patch

commit a4b31d5807f2bc67c8999b3d53369cf2a5c6e1ec (HEAD -> master) Author: Jakub Jelinek jakub@redhat.com Date: Sun Sep 27 23:18:26 2020 +0200

optabs: Don't reuse target for multi-word expansions if it overlaps operand(s) [PR97073]
kito-cheng commented 3 years ago

@shaochung Thanks your info! seems like the fix is already included in releases/gcc-10 branch, so it will included in gcc 10.3, and it should be released in next month.