Re: Question about long jump implementation
Hi Ian, thanks for the response.
I did manage to fix it. The problem was in my call_internal
implementation. I went back and looked again at the arm implementation
and spotted what I'd done wrong.
Sorry for the bother.
On 17 December 2010 01:21, Ian Lance Taylor wrote:
> Neil Hickey writes:
>
>> I'm trying to implement long jump and I'm using the call instruction
>> in the arm implementation as a reference.
>>
>> The code looks like this:
>>
>> rtx callee, pat;
>>
>> callee = XEXP(operands[0], 0);
>> if (GET_CODE (callee) == SYMBOL_REF
>> ? atdsp_is_long_call_p (SYMBOL_REF_DECL (callee))
>> : !REG_P (callee)) {
>> XEXP(operands[0], 0) = force_reg (SImode, callee);
>> }
>> pat = gen_call_internal (operands[0], operands[1]);
>> emit_call_insn(pat);
>> DONE;
>>
>> What I want to happen is for the function name to be copied to a
>> register and then the compiler emit the instruction to call the
>> function, by jumping to the address held in that register.
>>
>> The compiler emits instructions that copy the function name to a
>> register, then it loads from the value of this register, before
>> jumping to the loaded value. What I want it to do is copy the function
>> name to the register, then jump to the address held in this register.
>>
>> Does anyone know what it is I've done wrong and how I can get this to work?
>
> I think it's impossible for us to say without more information.
>
> Does the RTL look correct? If not, then you need to fix the RTL
> generation, which will look more or less like the code above. If the
> RTL does look correct, then you need to fix your MD file to generate
> instructions which correctly represent the RTL.
>
> Ian
>
RE: Is eliminate_regs_in_insn allowed to generate invalid instruction?
Ian,
Thanks for the reply.
Basically GCC generates an unrecognizable instruction during reload
src/weighted_prediction.c:815:1: error: unrecognizable insn:
(insn 1716 1715 680 84 src/weighted_prediction.c:729 (set (reg:SI 4 r4)
(plus:SI (mult:SI (reg:SI 9 r9)
(const_int 8 [0x8]))
(reg:SI 4 r4))) -1 (nil))
from gen_reload function.
/* Otherwise, just write (set OUT IN) and hope for the best. */
else
emit_insn (gen_rtx_SET (VOIDmode, out, in));
The comment doesn’t sound very convincing to me.
From debug message:
Reloads for insn # 680
Reload 0: reload_in (SI) = (plus:SI (reg/f:SI 57 r57)
(const_int 40 [0x28]))
GR_REGS, RELOAD_FOR_INPUT_ADDRESS (opnum = 1)
reload_in_reg: (plus:SI (reg/f:SI 57 r57)
(const_int 40 [0x28]))
reload_reg_rtx: (reg:SI 4 r4)
Reload 1: reload_in (SI) = (plus:SI (reg/f:SI 57 r57)
(const_int 78396 [0x1323c]))
GR_REGS, RELOAD_FOR_INPUT_ADDRESS (opnum = 1), can't combine
reload_in_reg: (plus:SI (reg/f:SI 57 r57)
(const_int 78396 [0x1323c]))
reload_reg_rtx: (reg:SI 6 r6)
Reload 2: reload_in (SI) = (mem/c:SI (plus:SI (reg/f:SI 57 r57)
(const_int 78396
[0x1323c])) [50 %sfp+78252 S4 A32])
GR_REGS, RELOAD_FOR_INPUT_ADDRESS (opnum = 1), can't combine
reload_in_reg: (reg:SI 596 [ ivtmp.474 ])
reload_reg_rtx: (reg:SI 9 r9)
Reload 3: reload_in (SI) = (plus:SI (mult:SI (reg:SI 596 [ ivtmp.474 ])
(const_int 8 [0x8]))
(plus:SI (reg/f:SI 57 r57)
(const_int 40 [0x28])))
GR_REGS, RELOAD_FOR_INPUT (opnum = 1), inc by 8
reload_in_reg: (plus:SI (mult:SI (reg:SI 596 [ ivtmp.474 ])
(const_int 8 [0x8]))
(plus:SI (reg/f:SI 57 r57)
(const_int 40 [0x28])))
reload_reg_rtx: (reg:SI 4 r4)
I don’t understand why Reload 3 is necessary. After reload 0, 1, 2,
The following expression already fits into our addressing mode.
(plus:SI (mult:SI (reg:SI 9 r9) (const_int 8 [0x8]))
(reg:SI 4 r4))
Instead GCC tries to generate invalid
(insn 1716 1715 680 84 src/weighted_prediction.c:729 (set (reg:SI 4 r4)
(plus:SI (mult:SI (reg:SI 9 r9)
(const_int 8 [0x8]))
(reg:SI 4 r4))) -1 (nil))
and load from [r4] subsequently.
Bingfeng
> -Original Message-
> From: Ian Lance Taylor [mailto:i...@google.com]
> Sent: 17 December 2010 01:26
> To: Bingfeng Mei
> Cc: gcc@gcc.gnu.org
> Subject: Re: Is eliminate_regs_in_insn allowed to generate invalid
> instruction?
>
> "Bingfeng Mei" writes:
>
> > I was hit by an ICE in reload. You know how difficult to debug it ☺.
> >
> > My primary suspect is that eliminate_regs_in_insn transforms
> >
> >
> > (insn 680 679 681 84 src/weighted_prediction.c:729 (set (reg:DF 1 r1)
> > (mem:DF (plus:SI (mult:SI (reg:SI 596 [ ivtmp.474 ])
> > (const_int 8 [0x8]))
> > (reg/f:SI 1105)) [3 S8 A8])) 448 {*ldl_dfmode} (nil))
> >
> >
> > To:
> > (insn 680 679 681 84 src/weighted_prediction.c:729 (set (reg:DF 1 r1)
> > (mem:DF (plus:SI (plus:SI (mult:SI (reg:SI 596 [ ivtmp.474 ])
> > (const_int 8 [0x8]))
> > (reg/f:SI 57 r57))
> > (const_int 40 [0x28])) [3 S8 A8])) 448 {*ldl_dfmode}
> (nil))
> >
> > The latter has illegal memory address mode, and r57 is our stack
> pointer.
> >
> > Since reload part is still half mystery to me, I wonder whether this
> is an
> > wrong transformation and causes the following ICE. Is it allowed that
> > Eliminate_regs_in_insn to generate such illegal instruction?
>
> It's normal for reload to produce instructions with invalid operands
> during its operation, with the expectation that the invalid operands
> will be reloaded into registers before the pass is complete. I
> couldn't
> see the specific ICE in your message, so I don't know if that is what
> is
> happening here.
>
> Ian
Question about ssa-form in gcc
Hi,
noticed, that after converting into ssa-form, there are still
variables, which are not wrapped into SSA_NAME.
Assigment to scalar variable:
int a;
a = 100;
The variable a is not wrapped into SSA_NAME a_1.
b = foo();
is converted into
b.0_3 = foo ();
# .MEM_25 = VDEF <.MEM_24>
b = b.0_3;
Question:
What is the purpose of this behavour? In classical ssa-representation
all variables should be numbered.
I guess it has somethink to do with memory-ssa and alias analysis.
Could anybody confirm or negate?
A simple program and the cut-off of the dump after putting into ssa follows.
int foo(){
return 100;
}
int main(int argc, char **args){
int a,b,d, *pa;
a=10;
b=20;
if(argc>3){
b=foo();
pa=&a;
b=*pa + 3;
}
else{
a+=1;
pa=&b;
}
*pa += 100;
b = a + b;
a++;
printf("*pa=%d\n",*pa+100);
return 0;
}
;; Function foo (foo)
foo ()
{
int D.2572;
:
D.2572_1 = 100;
return D.2572_1;
}
main (int argc, char * * args)
{
...
:
# .MEM_23 = VDEF <.MEM_22(D)>
a = 10;
# .MEM_24 = VDEF <.MEM_23>
b = 20;
if (argc_2(D) > 3)
goto ;
else
goto ;
:
b.0_3 = foo ();
# .MEM_25 = VDEF <.MEM_24>
b = b.0_3;
pa_4 = &a;
# VUSE <.MEM_25>
D.2561_5 = *pa_4;
b.1_6 = D.2561_5 + 3;
# .MEM_26 = VDEF <.MEM_25>
b = b.1_6;
goto ;
...
}
INIT_CUMULATIVE_ARGS (Was:Re: RFA: PR44566: accessor macros (1/3): CUMULATIVE_ARGS)
Quoting Nathan Froyd :
The grand plan (I was waiting until I had a patch to talk about this,
but since you asked...), looks something like this:
- Hookize FUNCTION_ARG &co. (patch approved, I need to reassure myself
that some of the const-ization in the patch isn't going to break
bootstrap and commit)
- Hookize INIT_CUMULATIVE_ARGS &co. INIT_CUMULATIVE_ARGS is the big
one; I plan to convert it to something with prototype:
CUMULATIVE_ARGS *target_init_cum_args (fntype, libname, fndecl, n_named);
As has been discussed in the meantime, we need a target-independent, opaque
type for the hook interface, which can be cumulative_args_t, typedefed to a
struct or union pointer, and converted from/to the target specific type in
a pair of small inline functions.
...
There is one small problem with the above plan (that I am aware of;
perhaps there are others that I am not aware of, and I would welcome
pointers to them): what to do with managing the 'CUMULATIVE_ARGS *'
returned by the backend? I think the options are:
- Have every backend return a pointer to statically allocated memory.
This is the easiest thing to do, but also likely to blow up for weird
uses of CUMULATIVE_ARGS. No need to free or equivalent. This is what
I was going to do.
That will be most efficient for the majority us uses, and thus should be
supported.
- Allocate it in the backend hook. Presumably we want to dictate the
method of allocation so we don't need to add *another* hook for
deallocating. obstacks, xmalloc, whatever. I don't have an opinion
here.
The problem is that that this doesn't allow you to use even alloca, so
you'd need a high-overhead allocation function.
I think it is better to have a piece-of-data target "hook" for the size
to allocate for a cumulative_args_t. Most or all of the places that can't
use static allocation can then use alloca.
Then the init_cumulative_args hook takes a void * argument for the allocated
memory. Targets that are fine with the statically allocated scheme can
pass in NULL.
DEFHOOKPOD
(cumulative_args_size,
"The amount of memory, in bytes, that the caller should allocate when\
passing a n...@code{null} @var{mem} argument to the\
@code{init_cumulative_args} target hook.",
size_t, sizeof (int))
DEFHOOK
(init_cumulative_args_args,
"", /* Still waiting on the FSF to allow us to put the documentation
here. */
cumulative_args_t,
(void *mem, tree fntype, rtx libname, tree fndecl, int n_named),
default_init_cumulative_args)
Re: Question about ssa-form in gcc
On Fri, Dec 17, 2010 at 06:00, Eugen Wagner wrote: > int a; > a = 100; > The variable a is not wrapped into SSA_NAME a_1. Because 'a' is a "memory variable". Memory is never rewritten into SSA form. To represent memory, we use factored use-def chains (the VDEF/VUSE notation you see in the dumps). Look in http://gcc.gnu.org/wiki/GettingStarted for articles/slides describing the memory SSA representation and aliasing information. > I guess it has somethink to do with memory-ssa and alias analysis. > Could anybody confirm or negate? Correct. You are looking at memory SSA information. Diego.
[pph] Mainline merge @167963
A couple of conflicts in the merge, but nothing too big. Bootstrapped and tested on x86_64. gcc/ChangeLog.pph Mainline merge r167963. * BASE-VER: Update revision stamp. libcpp/ChangeLog.pph * internal.h (struct cpp_lookaside): Rename field flag_pth_debug to pth_debug_level. gcc/cp/ChangeLog.pph * pph.c (pph_find_exposed_for, pph_get_decl_exposure): Replace uses of DECL_INTEGRAL_CONSTANT_VAR_P with decl_constant_var_p. Diego.
Re: Is eliminate_regs_in_insn allowed to generate invalid instruction?
"Bingfeng Mei" writes: > from gen_reload function. > /* Otherwise, just write (set OUT IN) and hope for the best. */ > else > emit_insn (gen_rtx_SET (VOIDmode, out, in)); Those lines are one of the curses of reload. When you hit them, you know something has gone wrong. Unfortunately, exactly what has gone wrong can be difficult to determine. It basically means that you are trying to reload something that the reload pass does not understand. > The comment doesn’t sound very convincing to me. > > > From debug message: > Reloads for insn # 680 > Reload 0: reload_in (SI) = (plus:SI (reg/f:SI 57 r57) > (const_int 40 [0x28])) > GR_REGS, RELOAD_FOR_INPUT_ADDRESS (opnum = 1) > reload_in_reg: (plus:SI (reg/f:SI 57 r57) > (const_int 40 [0x28])) > reload_reg_rtx: (reg:SI 4 r4) > Reload 1: reload_in (SI) = (plus:SI (reg/f:SI 57 r57) > (const_int 78396 > [0x1323c])) > GR_REGS, RELOAD_FOR_INPUT_ADDRESS (opnum = 1), can't combine > reload_in_reg: (plus:SI (reg/f:SI 57 r57) > (const_int 78396 > [0x1323c])) > reload_reg_rtx: (reg:SI 6 r6) > Reload 2: reload_in (SI) = (mem/c:SI (plus:SI (reg/f:SI 57 r57) > (const_int 78396 > [0x1323c])) [50 %sfp+78252 S4 A32]) > GR_REGS, RELOAD_FOR_INPUT_ADDRESS (opnum = 1), can't combine > reload_in_reg: (reg:SI 596 [ ivtmp.474 ]) > reload_reg_rtx: (reg:SI 9 r9) > Reload 3: reload_in (SI) = (plus:SI (mult:SI (reg:SI 596 [ ivtmp.474 ]) > (const_int 8 [0x8])) > (plus:SI (reg/f:SI 57 r57) > (const_int 40 > [0x28]))) > GR_REGS, RELOAD_FOR_INPUT (opnum = 1), inc by 8 > reload_in_reg: (plus:SI (mult:SI (reg:SI 596 [ ivtmp.474 ]) > (const_int 8 [0x8])) > (plus:SI (reg/f:SI 57 r57) > (const_int 40 > [0x28]))) > reload_reg_rtx: (reg:SI 4 r4) > > > I don’t understand why Reload 3 is necessary. After reload 0, 1, 2, > The following expression already fits into our addressing mode. > > (plus:SI (mult:SI (reg:SI 9 r9) (const_int 8 [0x8])) > (reg:SI 4 r4)) > > Instead GCC tries to generate invalid > (insn 1716 1715 680 84 src/weighted_prediction.c:729 (set (reg:SI 4 r4) > (plus:SI (mult:SI (reg:SI 9 r9) > (const_int 8 [0x8])) > (reg:SI 4 r4))) -1 (nil)) > and load from [r4] subsequently. Sounds like you're well on the way to tracking down the problem: you need to see why gcc decided to add reload 3 given the existence of the reloads 0 through 2. In particular, look at the code after if (strict_memory_address_addr_space_p (mode, ad, as)) in find_reloads_address. Ian
RE: Is eliminate_regs_in_insn allowed to generate invalid instruction?
Hi, Ian, I think I found the cause. find_reloads_address returns 0 even it reloads both parts of address (see the patch). Therefore, corresponding address_reloaded[i] is not set and in the following code of find_reloads, if (EXTRA_CONSTRAINT_STR (operand, c, p)) win = 1; /* If the address was already reloaded, we win as well. */ else if (MEM_P (operand) && address_reloaded[i] == 1) <-- address_reloaded[i] still 0 win = 1; ... Extra reload is thus generated even it is unnecessary and causes ICE. It looks like a general GCC bug. But I couldn't produce a test for x86. The following patch is bootstrapped and passes tests on x86_64-unknown-linux-gnu. Is it OK to apply the patch? Cheers, Bingfeng Index: reload.c === --- reload.c(revision 167979) +++ reload.c(working copy) @@ -5188,7 +5188,7 @@ find_reloads_address (enum machine_mode &XEXP (ad, 1 - op_index), opnum, type, 0, insn); - return 0; + return 1; } } > -Original Message- > From: Ian Lance Taylor [mailto:i...@google.com] > Sent: 17 December 2010 15:10 > To: Bingfeng Mei > Cc: gcc@gcc.gnu.org > Subject: Re: Is eliminate_regs_in_insn allowed to generate invalid > instruction? > > "Bingfeng Mei" writes: > > > from gen_reload function. > > /* Otherwise, just write (set OUT IN) and hope for the best. */ > > else > > emit_insn (gen_rtx_SET (VOIDmode, out, in)); > > Those lines are one of the curses of reload. When you hit them, you > know something has gone wrong. Unfortunately, exactly what has gone > wrong can be difficult to determine. It basically means that you are > trying to reload something that the reload pass does not understand. > > > > The comment doesn’t sound very convincing to me. > > > > > > From debug message: > > Reloads for insn # 680 > > Reload 0: reload_in (SI) = (plus:SI (reg/f:SI 57 r57) > > (const_int 40 > [0x28])) > > GR_REGS, RELOAD_FOR_INPUT_ADDRESS (opnum = 1) > > reload_in_reg: (plus:SI (reg/f:SI 57 r57) > > (const_int 40 > [0x28])) > > reload_reg_rtx: (reg:SI 4 r4) > > Reload 1: reload_in (SI) = (plus:SI (reg/f:SI 57 r57) > > (const_int 78396 > [0x1323c])) > > GR_REGS, RELOAD_FOR_INPUT_ADDRESS (opnum = 1), can't combine > > reload_in_reg: (plus:SI (reg/f:SI 57 r57) > > (const_int 78396 > [0x1323c])) > > reload_reg_rtx: (reg:SI 6 r6) > > Reload 2: reload_in (SI) = (mem/c:SI (plus:SI (reg/f:SI 57 r57) > > (const_int > 78396 [0x1323c])) [50 %sfp+78252 S4 A32]) > > GR_REGS, RELOAD_FOR_INPUT_ADDRESS (opnum = 1), can't combine > > reload_in_reg: (reg:SI 596 [ ivtmp.474 ]) > > reload_reg_rtx: (reg:SI 9 r9) > > Reload 3: reload_in (SI) = (plus:SI (mult:SI (reg:SI 596 > [ ivtmp.474 ]) > > (const_int 8 > [0x8])) > > (plus:SI > (reg/f:SI 57 r57) > > (const_int 40 > [0x28]))) > > GR_REGS, RELOAD_FOR_INPUT (opnum = 1), inc by 8 > > reload_in_reg: (plus:SI (mult:SI (reg:SI 596 [ ivtmp.474 ]) > > (const_int 8 > [0x8])) > > (plus:SI > (reg/f:SI 57 r57) > > (const_int 40 > [0x28]))) > > reload_reg_rtx: (reg:SI 4 r4) > > > > > > I don’t understand why Reload 3 is necessary. After reload 0, 1, 2, > > The following expression already fits into our addressing mode. > > > > (plus:SI (mult:SI (reg:SI 9 r9) (const_int 8 [0x8])) > > (reg:SI 4 r4)) > > > > Instead GCC tries to generate invalid > > (insn 1716 1715 680 84 src/weighted_prediction.c:729 (set (reg:SI 4 > r4) > > (plus:SI (mult:SI (reg:SI 9 r9) > > (const_int 8 [0x8])) > > (reg:SI 4 r4))) -1 (nil)) > > and load from [r4] subsequently. > > Sounds like you're well on the way to tracking down the problem: you > need to see why gcc decided to add reload 3 given the existence of the > reloads 0 through 2. In particular, look at the code after > if (strict_memory_address_addr_space_p (mode, ad, as)) > in find_reloads_address. > > Ian
Re: Is eliminate_regs_in_insn allowed to generate invalid instruction?
"Bingfeng Mei" writes: > I think I found the cause. find_reloads_address returns 0 even > it reloads both parts of address (see the patch). Therefore, > corresponding address_reloaded[i] is not set and in > the following code of find_reloads, > >if (EXTRA_CONSTRAINT_STR (operand, c, p)) > win = 1; > /* If the address was already reloaded, >we win as well. */ >else if (MEM_P (operand) > && address_reloaded[i] == 1) <-- address_reloaded[i] still 0 > win = 1; > ... > > Extra reload is thus generated even it is unnecessary > and causes ICE. > > It looks like a general GCC bug. But I couldn't produce > a test for x86. The following patch is bootstrapped > and passes tests on x86_64-unknown-linux-gnu. Is it > OK to apply the patch? > > Cheers, > Bingfeng > > > Index: reload.c > === > --- reload.c(revision 167979) > +++ reload.c(working copy) > @@ -5188,7 +5188,7 @@ find_reloads_address (enum machine_mode > &XEXP (ad, 1 - op_index), opnum, > type, 0, insn); > > - return 0; > + return 1; I'm willing to believe that there is a problem here, but that patch isn't right. find_reloads_address should only return 1 if the address is replaced or reloaded as a whole, and that is not what is happening here. What is happening here is that the components of the address are being reloaded. Frankly I would fix this problem in LEGITIMIZE_RELOAD_ADDRESS. Ian
Re: Is eliminate_regs_in_insn allowed to generate invalid instruction?
Bingfeng Mei wrote: > I think I found the cause. find_reloads_address returns 0 even > it reloads both parts of address (see the patch). Therefore, > corresponding address_reloaded[i] is not set and in > the following code of find_reloads, > >if (EXTRA_CONSTRAINT_STR (operand, c, p)) > win = 1; > /* If the address was already reloaded, >we win as well. */ >else if (MEM_P (operand) > && address_reloaded[i] == 1) <-- address_reloaded[i] still 0 > win = 1; > ... > > Extra reload is thus generated even it is unnecessary > and causes ICE. As Ian said, since the address wasn't *completely* reloaded (so that after reload we'll have just a plain base register), it is correct that address_reloaded must be 0. However, if you're running into problems in this part of the code, you may probably hit another of the long-standing warts in reload: at this point, you have an address that reload has already decided to reload certain parts of, and the result has to match one of your port's extra memory constraints. However, reload at this point has not actually made any modifications to the address in its RTL form, it has just recorded in its own tables that *later*, it *will* replace some subexpressions of that RTL with registers. This means that the RTL that is passed to your EXTRA_CONSTRAINT_STR implementation will still be the *original* un-reloaded address. And most likely, your back-end will then reject this address as not valid for your machine. The way some ports take around this issue is to recognize, in your EXTRA_CONSTRAINT_STR implementation, certain forms of complex addresses as those which you *know* reload will already have marked for reloading, and therefore *accept* them (if they'd otherwise match the constraint). Bye, Ulrich -- Dr. Ulrich Weigand GNU Toolchain for Linux on System z and Cell BE ulrich.weig...@de.ibm.com
Re: Is eliminate_regs_in_insn allowed to generate invalid instruction?
"Ulrich Weigand" writes: > The way some ports take around this issue is to recognize, in your > EXTRA_CONSTRAINT_STR implementation, certain forms of complex > addresses as those which you *know* reload will already have marked > for reloading, and therefore *accept* them (if they'd otherwise > match the constraint). Seems like you could also just use find_replacement if reload_in_progress is true. Ian
Re: Is eliminate_regs_in_insn allowed to generate invalid instruction?
Ian Lance Taylor wrote: > "Ulrich Weigand" writes: > > The way some ports take around this issue is to recognize, in your > > EXTRA_CONSTRAINT_STR implementation, certain forms of complex > > addresses as those which you *know* reload will already have marked > > for reloading, and therefore *accept* them (if they'd otherwise > > match the constraint). > > Seems like you could also just use find_replacement if > reload_in_progress is true. Unfortunately that doesn't work, because find_replacement can only be used during the later phases of reload, where find_reloads was called with a nonzero REPLACE parameter. During the early phases, where find_reloads is called with zero as REPLACE parameter, the replacements structure is not filled in, and find_replacement does not work. A back-end memory constraint callback also needs to work during those early phases, however ... Bye, Ulrich -- Dr. Ulrich Weigand GNU Toolchain for Linux on System z and Cell BE ulrich.weig...@de.ibm.com
