[Bug target/103271] ICE in assign_stack_temp_for_type with -ftrivial-auto-var-init=pattern and VLAs and -mno-strict-align on riscv64
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=103271 Jim Wilson changed: What|Removed |Added CC||kito.cheng at gmail dot com --- Comment #18 from Jim Wilson --- I found the place where the movti patterns were removed. This is a riscv-gcc commit from Kito. commit 38650bdbe91bf37c3cce706ce612097b657a91d5 Author: Kito Cheng Date: Mon Sep 12 14:52:56 2016 +0800 Remove 128 bit support, just let gcc handle it I don't see a github pull request or issue for it, so I don't know why Kito removed the support. This is a little too long ago to easily find info related to the change. This is a gcc-6.1 source base. Unless perhaps Kito remembers why he made the change, all I can do is readd the support and wait to see if something else breaks. I do think that we should readd the movti support to fix a couple of bugs. I suspect a code size or performance issue, but wrong code and ICEs are worse problems than code size and performance.
[Bug target/103271] ICE in assign_stack_temp_for_type with -ftrivial-auto-var-init=pattern and VLAs and -mno-strict-align on riscv64
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=103271
--- Comment #17 from qinzhao at gcc dot gnu.org ---
(In reply to Richard Biener from comment #12)
> diff --git a/gcc/internal-fn.c b/gcc/internal-fn.c
> index 6ac3460d538..08f94b7a17a 100644
> --- a/gcc/internal-fn.c
> +++ b/gcc/internal-fn.c
> @@ -3050,6 +3050,23 @@ expand_DEFERRED_INIT (internal_fn, gcall *stmt)
> lhs_base = TREE_OPERAND (lhs_base, 0);
>reg_lhs = (mem_ref_refers_to_non_mem_p (lhs_base)
> || non_mem_decl_p (lhs_base));
> + /* If this expands to a register and the underlying decl is wrapped in
> +a MEM_REF that just serves as an access type change expose the decl
> +if it is of correct size. This avoids a situation as in PR103271
> +if the target does not support a direct move to the registers mode.
> */
> + if (reg_lhs
> + && TREE_CODE (lhs_base) == MEM_REF
> + && TREE_CODE (TREE_OPERAND (lhs_base, 0)) == ADDR_EXPR
> + && DECL_P (TREE_OPERAND (TREE_OPERAND (lhs_base, 0), 0))
> + && integer_zerop (TREE_OPERAND (lhs_base, 1))
> + && tree_fits_uhwi_p (var_size)
> + && tree_int_cst_equal
> + (var_size,
> + DECL_SIZE_UNIT (TREE_OPERAND (TREE_OPERAND (lhs_base, 0),
> 0
> + {
> + lhs = TREE_OPERAND (TREE_OPERAND (lhs_base, 0), 0);
> + var_type = TREE_TYPE (lhs);
> + }
> }
>
>if (!reg_lhs)
>
> solves the issue. There might be a situation where that's not enough though,
> I'm giving the above some testing.
this patch is so specific to target this specific case, I am wondering whether
this is needed if Tim will provide a patch to support MOVTI in riscv backend?
[Bug target/103271] ICE in assign_stack_temp_for_type with -ftrivial-auto-var-init=pattern and VLAs and -mno-strict-align on riscv64
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=103271 --- Comment #16 from Jim Wilson --- I have a patch to add movti to the riscv port. I agree that we should be adding this. I just unfortunately had a kitchen accident and had take some time off before I finished it. I noticed that a comment before riscv_split_doubleword_move implies that we did used to have a movti pattern. I wanted to check out the history before re-adding movti to make sure I didn't break anything.
[Bug target/103271] ICE in assign_stack_temp_for_type with -ftrivial-auto-var-init=pattern and VLAs and -mno-strict-align on riscv64
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=103271 --- Comment #15 from qinzhao at gcc dot gnu.org --- (In reply to Richard Biener from comment #11) > > I suppose assigning TImode to a decl but not even being able to move TImode > can be a problem elsewhere... this might be the root issue that we should fix? as mentioned by Jim in comment #5 and #6, adding a MOVTI pattern in riscv will fix other bug in addition to this one. so, should the right fix be in riscv backend?
[Bug target/103271] ICE in assign_stack_temp_for_type with -ftrivial-auto-var-init=pattern and VLAs and -mno-strict-align on riscv64
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=103271 Richard Biener changed: What|Removed |Added Resolution|--- |FIXED Status|NEW |RESOLVED --- Comment #14 from Richard Biener --- The specific case is avoided now, still I'm unsure if riscv is "bad" in not providing moves to TImode pseudos.
[Bug target/103271] ICE in assign_stack_temp_for_type with -ftrivial-auto-var-init=pattern and VLAs and -mno-strict-align on riscv64
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=103271 --- Comment #13 from CVS Commits --- The master branch has been updated by Richard Biener : https://gcc.gnu.org/g:06b8cdc8d7339ac44802044ef148dd86874333d8 commit r12-5709-g06b8cdc8d7339ac44802044ef148dd86874333d8 Author: Richard Biener Date: Thu Dec 2 12:23:22 2021 +0100 middle-end/103271 - avoid VLA init of register This avoids using VLA types to initalize a register with -ftrivial-auto-var-init in some cases. 2021-12-02 Richard Biener PR middle-end/103271 * internal-fn.c (expand_DEFERRED_INIT): When the base of the LHS is a decl with matching constant size use that as the initialization target instead of an eventual VLA typed one.
[Bug target/103271] ICE in assign_stack_temp_for_type with -ftrivial-auto-var-init=pattern and VLAs and -mno-strict-align on riscv64
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=103271
--- Comment #12 from Richard Biener ---
diff --git a/gcc/internal-fn.c b/gcc/internal-fn.c
index 6ac3460d538..08f94b7a17a 100644
--- a/gcc/internal-fn.c
+++ b/gcc/internal-fn.c
@@ -3050,6 +3050,23 @@ expand_DEFERRED_INIT (internal_fn, gcall *stmt)
lhs_base = TREE_OPERAND (lhs_base, 0);
reg_lhs = (mem_ref_refers_to_non_mem_p (lhs_base)
|| non_mem_decl_p (lhs_base));
+ /* If this expands to a register and the underlying decl is wrapped in
+a MEM_REF that just serves as an access type change expose the decl
+if it is of correct size. This avoids a situation as in PR103271
+if the target does not support a direct move to the registers mode.
*/
+ if (reg_lhs
+ && TREE_CODE (lhs_base) == MEM_REF
+ && TREE_CODE (TREE_OPERAND (lhs_base, 0)) == ADDR_EXPR
+ && DECL_P (TREE_OPERAND (TREE_OPERAND (lhs_base, 0), 0))
+ && integer_zerop (TREE_OPERAND (lhs_base, 1))
+ && tree_fits_uhwi_p (var_size)
+ && tree_int_cst_equal
+ (var_size,
+ DECL_SIZE_UNIT (TREE_OPERAND (TREE_OPERAND (lhs_base, 0), 0
+ {
+ lhs = TREE_OPERAND (TREE_OPERAND (lhs_base, 0), 0);
+ var_type = TREE_TYPE (lhs);
+ }
}
if (!reg_lhs)
solves the issue. There might be a situation where that's not enough though,
I'm giving the above some testing.
[Bug target/103271] ICE in assign_stack_temp_for_type with -ftrivial-auto-var-init=pattern and VLAs and -mno-strict-align on riscv64
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=103271
Richard Biener changed:
What|Removed |Added
CC||ebotcazou at gcc dot gnu.org,
||law at gcc dot gnu.org
--- Comment #11 from Richard Biener ---
(In reply to qinzhao from comment #10)
> looks like that this is exactly the same issue as exposed in pr102285.
>
> and the original fix to pr102285 just hide this inconsistent IR issue.
>
> -mno-strict-align exposed this issue again.
>
> So. I believe that we need to fix the inconsistent IR issue in order to
> completely resolve this issue.
The issue is simply that using build_zero_cst is "wrong" here in the sense
that we know the size is constant but we hide that fact.
In fact, C disallows initializing variable-sized objects, so an adjusted
testcase with
int fb[tw] = {};
is not valid C. I think the situation with risc-v is unfortunate since
all other places in the compiler that zero non-memory simply use
emit_move_insn from CONST0_RTX but that's not avilable since we do not
go the "correct" path only because have_insn_for (SET, TImode) does not
exist.
But I'm not a target / RTL expert enough to tell whether that's a general
problem that should be fixed.
I also have no idea how to manually generate a proper zeroing sequence
for the (reg:TI ..) pseudo the VLA typed aggregate expands to.
I suppose assigning TImode to a decl but not even being able to move TImode
can be a problem elsewhere...
[Bug target/103271] ICE in assign_stack_temp_for_type with -ftrivial-auto-var-init=pattern and VLAs and -mno-strict-align on riscv64
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=103271 --- Comment #10 from qinzhao at gcc dot gnu.org --- looks like that this is exactly the same issue as exposed in pr102285. and the original fix to pr102285 just hide this inconsistent IR issue. -mno-strict-align exposed this issue again. So. I believe that we need to fix the inconsistent IR issue in order to completely resolve this issue.
[Bug target/103271] ICE in assign_stack_temp_for_type with -ftrivial-auto-var-init=pattern and VLAs and -mno-strict-align on riscv64
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=103271 --- Comment #9 from qinzhao at gcc dot gnu.org --- disable tree-ccp by adding -fno-tree-ccp cures the ICE.
[Bug target/103271] ICE in assign_stack_temp_for_type with -ftrivial-auto-var-init=pattern and VLAs and -mno-strict-align on riscv64
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=103271 --- Comment #8 from qinzhao at gcc dot gnu.org --- the minimum option to repeat this failure is "-O1 -mno-strict-align". The option "-mno-strict-align" is the one that make the difference. For the following call to .DEFERRED_INIT: MEM[(int[0:D.1492] *)] = .DEFERRED_INIT (16, 1, 1); the LHS is MEM[(int[0:D.1492] *)]. When -mno-strict-align is NOT present, "mem_ref_refers_to_non_mem_p (lhs_base)" return FALSE, lHS is considered as MEM, and the call is expanded through "memset" path. No issue. when -mno-strict-align is present, "mem_ref_refers_to_non_mem_p (lhs_base)" return TRUE, as a result, LHS is considered to be put into a register, and then the call is expanded through "expand_assignment" path. The major issue during expanding through "expand_assignment" path is: although the LHS "MEM[(int[0:D.1492] *)]" is decided to be put into register with TI mode, however, the TREE_TYPE of LHS is still a VLA type, such inconsistency in IR cause the final ICE. with the option -mno-strict-align, the IR of lhs_base is: (gdb) call debug_tree(lhs_base) unit-size align:32 warn_if_not_align:0 symtab:0 alias-set 1 canonical-type 0x707465e8 precision:32 min max pointer_to_this > sizes-gimplified type_1 BLK size used unsigned ignored TI t.c:8:7 size unit-size align:128 warn_if_not_align:0 context > unit-size used unsigned ignored DI t.c:8:7 size unit-size align:64 warn_if_not_align:0 context > align:32 warn_if_not_align:0 symtab:0 alias-set -1 structural-equality domain sizes-gimplified DI size unit-size align:64 warn_if_not_align:0 symtab:0 alias-set -1 structural-equality precision:64 min max > pointer_to_this > nothrow arg:0 unsigned DI size unit-size align:64 warn_if_not_align:0 symtab:0 alias-set -1 canonical-type 0x7082c2a0> arg:0 used ignored TI t.c:8:7 size unit-size align:64 warn_if_not_align:0 context (reg:TI 72 [ fb.3 ])>> arg:1 constant 0> t.c:8:7 start: t.c:8:7 finish: t.c:8:8> >From the above IR, we can see, 1. the base address of this mem_ref is arg:0 used ignored TI t.c:8:7 size unit-size align:64 warn_if_not_align:0 context (reg:TI 72 [ fb.3 ])>> which is in register of TI mode; 2. However, the TREE_TYPE of this mem_ref is still a VLA type. when call "build_zero_cst (var_type)", the var_type is a VLA type, therefore the ICE. My suspicion is, -mno-strict-align applied some kind of optimization that turn the var_decl from: arg:0 used ignored BLK t.c:8:7 size unit-size align:128 warn_if_not_align:0 context (mem/c:BLK (plus:DI (reg/f:DI 67 virtual-stack-vars) (const_int -16 [0xfff0])) [0 fb.3+0 S16 A128])>> to: arg:0 used ignored TI t.c:8:7 size unit-size align:64 warn_if_not_align:0 context (reg:TI 72 [ fb.3 ])>> However, the TREE_TYPE of MEM[(int[0:D.1492] *)] is not updated accordingly.
[Bug target/103271] ICE in assign_stack_temp_for_type with -ftrivial-auto-var-init=pattern and VLAs and -mno-strict-align on riscv64
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=103271 --- Comment #7 from rguenther at suse dot de --- On Fri, 26 Nov 2021, wilson at gcc dot gnu.org wrote: > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=103271 > > Jim Wilson changed: > >What|Removed |Added > > CC||wilson at gcc dot gnu.org > > --- Comment #5 from Jim Wilson --- > SiFive doesn't support -mno-strict-align so I've never tested it. I doubt > that > it works correctly, i.e. I doubt that it optimizes as intended. I've > mentioned > this to other RVI members, but there hasn't been anyone other than SiFive > actively working on upstream gcc so I don't think anyone ever looked at it. > It > shouldn't give an ICE though. > > Looking at this, it appears to be another "if only we had a movti pattern" > issue. > > In expand_DEFERRED_INIT in internal-fn.c, in the reg_lhs == TRUE case, there > is > a test > && have_insn_for (SET, var_mode)) > which fails because var_mode is TImode and we don't have a movti pattern. The > code calls build_zero_cst which returns a constructor with an array type. We > then call expand_assignment which gets confused as it doesn't know the size of > the array it is copying. That seems to be the bug - in this path we shouldn't ever create an entity with VLA size since we do know the actual size. But it all is a bit awkward.
[Bug target/103271] ICE in assign_stack_temp_for_type with -ftrivial-auto-var-init=pattern and VLAs and -mno-strict-align on riscv64
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=103271 --- Comment #6 from Jim Wilson --- See also bug 103302 which can also be fixed by adding a movti pattern.
[Bug target/103271] ICE in assign_stack_temp_for_type with -ftrivial-auto-var-init=pattern and VLAs and -mno-strict-align on riscv64
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=103271 Jim Wilson changed: What|Removed |Added CC||wilson at gcc dot gnu.org --- Comment #5 from Jim Wilson --- SiFive doesn't support -mno-strict-align so I've never tested it. I doubt that it works correctly, i.e. I doubt that it optimizes as intended. I've mentioned this to other RVI members, but there hasn't been anyone other than SiFive actively working on upstream gcc so I don't think anyone ever looked at it. It shouldn't give an ICE though. Looking at this, it appears to be another "if only we had a movti pattern" issue. In expand_DEFERRED_INIT in internal-fn.c, in the reg_lhs == TRUE case, there is a test && have_insn_for (SET, var_mode)) which fails because var_mode is TImode and we don't have a movti pattern. The code calls build_zero_cst which returns a constructor with an array type. We then call expand_assignment which gets confused as it doesn't know the size of the array it is copying. However, if we had a movti pattern, then the code computes the size of the array, and creates a VIEW_CONVERT_EXPR to document the array size before calling expand_assignment. So it looks like it would work if we had a movti pattern. I verified that adding a dummy movti pattern makes the ICE go away.
[Bug target/103271] ICE in assign_stack_temp_for_type with -ftrivial-auto-var-init=pattern and VLAs and -mno-strict-align on riscv64
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=103271 --- Comment #4 from Qing Zhao --- > > You should be able to simply do > > ../configure --target=riscv64-linux > make all-gcc > > and use the built gcc/cc1 to debug such ICEs. > Thanks. Yes, I can repeat the ICE with this gcc even though “make install” still failed with the following error: make[2]: Leaving directory '/home/opc/Work/GCC/crossbuild/libiberty' /bin/sh: line 3: cd: ./c++tools: No such file or directory make[1]: *** [Makefile:12252: install-c++tools] Error 1 make[1]: Leaving directory '/home/opc/Work/GCC/crossbuild' make: *** [Makefile:2538: install] Error 2 Anyway, I can debug this current bug with this gcc now.
[Bug target/103271] ICE in assign_stack_temp_for_type with -ftrivial-auto-var-init=pattern and VLAs and -mno-strict-align on riscv64
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=103271 --- Comment #3 from rguenther at suse dot de --- On Mon, 22 Nov 2021, qinzhao at gcc dot gnu.org wrote: > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=103271 > > qinzhao at gcc dot gnu.org changed: > >What|Removed |Added > > CC||qinzhao at gcc dot gnu.org > > --- Comment #1 from qinzhao at gcc dot gnu.org --- > was not able to repeat this failure yet due to: > > 1. cannot find a riscv machine either in my company or in gcc farm. > 2. tried to build a cross-compiler on riscv64 from a x86 platform, but always > failed. > > is there a good documentation to build cross-compiler? You should be able to simply do ../configure --target=riscv64-linux make all-gcc and use the built gcc/cc1 to debug such ICEs.
[Bug target/103271] ICE in assign_stack_temp_for_type with -ftrivial-auto-var-init=pattern and VLAs and -mno-strict-align on riscv64
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=103271 --- Comment #2 from Martin Liška --- (In reply to qinzhao from comment #1) > was not able to repeat this failure yet due to: > > 1. cannot find a riscv machine either in my company or in gcc farm. > 2. tried to build a cross-compiler on riscv64 from a x86 platform, but > always failed. It's as simple as: $ ../configure --enable-languages=c,c++ --disable-multilib --disable-libsanitizer --disable-bootstrap --target=riscv64-linux-gnu $ make all-host > > is there a good documentation to build cross-compiler? https://gcc.gnu.org/install/build.html#Building-a-cross-compiler
[Bug target/103271] ICE in assign_stack_temp_for_type with -ftrivial-auto-var-init=pattern and VLAs and -mno-strict-align on riscv64
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=103271 qinzhao at gcc dot gnu.org changed: What|Removed |Added CC||qinzhao at gcc dot gnu.org --- Comment #1 from qinzhao at gcc dot gnu.org --- was not able to repeat this failure yet due to: 1. cannot find a riscv machine either in my company or in gcc farm. 2. tried to build a cross-compiler on riscv64 from a x86 platform, but always failed. is there a good documentation to build cross-compiler?
