On 13-08-06 3:11 PM, Richard Henderson wrote:
On 08/06/2013 03:57 AM, Kirill Yukhin wrote:
On 05 Aug 09:55, Richard Henderson wrote:
On 08/05/2013 08:07 AM, Kirill Yukhin wrote:
Hello Richard, Vlad,

On 31 Jul 06:26, Richard Henderson wrote:
On 07/31/2013 05:02 AM, Kirill Yukhin wrote:
There's ICE (max. number of generated reload insns per insn is achieved (90)),
when LRA tried to save mask register before call. This was caused by fact that  
split_reg function
in lra-constraints.c allocates memory for saved reg in 
SECONDARY_MEMORY_NEEDED_MODE which
I've told you before that it's not SECONDARY_MEMORY that you want, but
a secondary reload.  You should be patching ix86_secondary_reload, right
below where we handle QImode spills from non-Q registers for x86-32.
Trying to do that with no success so far.
Could you pls correct me if I am wrong.
What I am trying to do is to introduce 2 new `define_expand' for load and store.
Huh?  You shouldn't need this.

Give me a test case and I can have a look at it.
Hello,

I've squashed part 1 and 2 + rebased on recent trunk.
Testcase is attached.
To reproduce: build-x86_64-linux/gcc/xgcc -Bbuild-x86_64-linux/gcc -Ofast 
-mavx512f -march=core-avx2 repro.c  -S -o-  -ffixed-rsi  -ffixed-rdi  
-ffixed-rbx -ffixed-rbp -m32

Thanks a lot for help!
You've found what I believe to be a bug in LRA.

Specifically, lra-constraints.c split_reg uses SECONDARY_MEMORY_NEEDED_MODE to
choose what mode to spill a caller-save register.  Given the existing
definition in i386.h, this tries to spill a MASK_CLASS register in SImode.  But
MASK_CLASS does not support SImode, only QI/HImode.  Which leads to substantial
confusion in the allocator trying to satisfy the move.

I believe the use of SECONDARY_MEMORY_NEEDED_MODE in split_reg is wrong.
What's the history behind that, Vlad?  Surely we can spill the value in
its current mode?

As I remember I tried to decrease number of macros used for LRA.

Just using mode of reg might not work for general case.

Reload (caller-saves.c) uses HARD_REGNO_CALLER_SAVE_MODE. I guess we should use it.

I'll try to implement this and after some testing and checking on a few platform I'll commit it.

I guess we will have a solution at the end of this week.
Certainly this patch fixes the crash from Kirill's reproducer...

Thanks for working on this, Richard.


Reply via email to