> FWIW, x86_64-darwin *passes* the test-cases you added with the patch series.
gcc.target/i386/sibcall-1.c fails in 32 bit mode:
FAIL: gcc.target/i386/sibcall-1.c scan-assembler-not mov
Cheers,
Dominique
Hello Kai,
On 28 May 2014, at 09:43, Kai Tietz wrote:
> Index: gcc/config/i386/i386.c
> ===
> --- gcc/config/i386/i386.c(revision 210985)
> +++ gcc/config/i386/i386.c(working copy)
> @@ -38893,7 +38893,16 @@ x86_output_mi_thu
In testcase a NULL escaped ...
ChangeLog testsuite
2014-05-31 Kai Tietz
* gcc.target/i386/sibcall-6.c: New test.
Index: gcc.target/i386/sibcall-6.c
===
--- gcc.target/i386/sibcall-6.c(Revision 0)
+++ gcc.target/i386/sibc
Hello,
recent fallout about sibcall was caused by using 'm' constraint for
sibcalls. By this wrongly combines happened on reload-pass.
ChangeLog
2014-05-31 Kai Tietz
* constrains.md (define_constrain): New 'B' constraint.
* i386.md (sibcall_insn_operand): Use B instead of m constrai
That patch won't help here. My tests showing here a different
problem. For -O2 or above this issue happens.
For -O2 the following code will be generated.
movl28(%esp), %eax
testl %eax, %eax
je L16
movl%ebp, 64(%esp)
addl$44, %esp
Hmm, this might cause by allowing CONST-memory-addresses. This was
doubtful from the beginning.
It would be helpful to see here for the case a final rtl-dump.
Could you try if following patch (it is incomplete as it disregards
plus-expression) solves your bootstrap issue?
Regards,
Kai
Index: p
On 28-05-14 10:43, Kai Tietz wrote:
Index: gcc/testsuite/gcc.target/i386/sibcall-4.c
===
--- gcc/testsuite/gcc.target/i386/sibcall-4.c (revision 0)
+++ gcc/testsuite/gcc.target/i386/sibcall-4.c (working copy)
@@ -0,0 +1,15 @@
+/*
Hi Kai,
this patch also mis-compiles binuitls-2.24 on x86_64.
In the function walk_wild_consider_section (ld/ldlang.c)
a tail-call gets miscompiled:
The stack frame is cleaned up, but now the jump target is invalid.
0x0040c801 <+193>: add $0x28,%rsp
0x0040c805 <+197>
On Fri, May 30, 2014 at 3:05 PM, H.J. Lu wrote:
> On Fri, May 30, 2014 at 9:51 AM, Kai Tietz wrote:
>> - Original Message -
>>> On 05/30/2014 01:08 AM, Kai Tietz wrote:
>>> > (define_predicate "sibcall_memory_operand"
>>> > (match_operand 0 "memory_operand")
>>> > {
>>> > return CONST
On Fri, May 30, 2014 at 9:51 AM, Kai Tietz wrote:
> - Original Message -
>> On 05/30/2014 01:08 AM, Kai Tietz wrote:
>> > (define_predicate "sibcall_memory_operand"
>> > (match_operand 0 "memory_operand")
>> > {
>> > return CONSTANT_P (op);
>> > })
>>
>> Surely that always returns fals
On 05/30/14 10:51, Kai Tietz wrote:
- Original Message -
On 05/30/2014 01:08 AM, Kai Tietz wrote:
(define_predicate "sibcall_memory_operand"
(match_operand 0 "memory_operand")
{
return CONSTANT_P (op);
})
Surely that always returns false? Surely XEXP (op, 0) so that you look at
- Original Message -
> On 05/30/2014 01:08 AM, Kai Tietz wrote:
> > (define_predicate "sibcall_memory_operand"
> > (match_operand 0 "memory_operand")
> > {
> > return CONSTANT_P (op);
> > })
>
> Surely that always returns false? Surely XEXP (op, 0) so that you look at
> the
> address,
On 05/30/2014 01:08 AM, Kai Tietz wrote:
> (define_predicate "sibcall_memory_operand"
> (match_operand 0 "memory_operand")
> {
> return CONSTANT_P (op);
> })
Surely that always returns false? Surely XEXP (op, 0) so that you look at the
address, not the memory.
r~
So, completed bootstrap and regression-test for x86_64-unknown-linux-gnu
(multilib) by using following predicate for sibcall-patch.
(define_predicate "sibcall_memory_operand"
(match_operand 0 "memory_operand")
{
return CONSTANT_P (op);
})
Worked fine, no regressions. Is sibcall-patch ok wi
On 05/28/2014 02:54 PM, Jeff Law wrote:
> On 05/28/14 15:52, Jakub Jelinek wrote:
>> On Wed, May 28, 2014 at 05:28:31PM -0400, Kai Tietz wrote:
>>> Yes, I missed the plus-part.
>>>
>>> I am just running bootstrap with regression testing for altering predicate
>>> to:
>>>
>>> (define_predicate "sib
- Original Message -
> On Wed, May 28, 2014 at 03:54:58PM -0600, Jeff Law wrote:
> > >Why not get rid of all the above 4 lines and just keep:
> > >
> > >> return CONSTANT_P (op);
> > >
> > >? CONST matches CONSTANT_P, and what is inside of CONST should be
> > >fine, and (plus (symbol_ref
On Wed, May 28, 2014 at 03:54:58PM -0600, Jeff Law wrote:
> >Why not get rid of all the above 4 lines and just keep:
> >
> >> return CONSTANT_P (op);
> >
> >? CONST matches CONSTANT_P, and what is inside of CONST should be
> >fine, and (plus (symbol_ref) (const_int)) not surrounded by CONST
> >i
On 05/28/14 15:52, Jakub Jelinek wrote:
On Wed, May 28, 2014 at 05:28:31PM -0400, Kai Tietz wrote:
Yes, I missed the plus-part.
I am just running bootstrap with regression testing for altering predicate to:
(define_predicate "sibcall_memory_operand"
(match_operand 0 "memory_operand")
{
o
On Wed, May 28, 2014 at 05:28:31PM -0400, Kai Tietz wrote:
> Yes, I missed the plus-part.
>
> I am just running bootstrap with regression testing for altering predicate to:
>
> (define_predicate "sibcall_memory_operand"
> (match_operand 0 "memory_operand")
> {
> op = XEXP (op, 0);
>
> if (
- Original Message -
> On 05/28/14 11:22, Richard Henderson wrote:
> > On 05/28/2014 01:43 AM, Kai Tietz wrote:
> >> + if (GET_CODE (op) == CONST)
> >> +op = XEXP (op, 0);
> >> + return (GET_CODE (op) == SYMBOL_REF || CONSTANT_P (op));
> >
> > Surely all this boils down to just CONS
On 05/28/14 11:22, Richard Henderson wrote:
On 05/28/2014 01:43 AM, Kai Tietz wrote:
+ if (GET_CODE (op) == CONST)
+op = XEXP (op, 0);
+ return (GET_CODE (op) == SYMBOL_REF || CONSTANT_P (op));
Surely all this boils down to just CONSTANT_P (op),
without having to look through the CONST a
On 05/28/2014 01:43 AM, Kai Tietz wrote:
> + if (GET_CODE (op) == CONST)
> +op = XEXP (op, 0);
> + return (GET_CODE (op) == SYMBOL_REF || CONSTANT_P (op));
Surely all this boils down to just CONSTANT_P (op),
without having to look through the CONST at all.
Otherwise this looks ok.
r~
On Wed, May 28, 2014 at 1:43 AM, Kai Tietz wrote:
> Hi,
>
> I modified prior patch so that it uses the new predicate
> sibcall_memory_operand to extend sibcall_insn_operand.
> Just one change in i386.c remains about x86_output_mi_thunk. Later one isn't
> pretty much essential. Nevertheless it
Hi,
I modified prior patch so that it uses the new predicate sibcall_memory_operand
to extend sibcall_insn_operand.
Just one change in i386.c remains about x86_output_mi_thunk. Later one isn't
pretty much essential. Nevertheless it makes
code equivalent to none-memory-case for potential tail-s
On 05/27/2014 09:48 AM, Jeff Law wrote:
>> leaofs(base, index, scale), %eax
>> ...
>> call*0(%eax)
>>
>> we might as well include the memory load
>>
>> movofs(base, index, scale), %eax
>> ...
>> call*%eax
> Ok. My misunderstanding.
>
> Granted, this probabl
On 05/27/14 10:22, Richard Henderson wrote:
On 05/27/2014 08:39 AM, Jeff Law wrote:
But the value we want may be sitting around in a convenient register (such as
r11). So if we force the sibcall to use %rax, then we have to generate a
copy. Yet if we have a constraint for the set of registers
On 05/27/2014 08:39 AM, Jeff Law wrote:
> But the value we want may be sitting around in a convenient register (such as
> r11). So if we force the sibcall to use %rax, then we have to generate a
> copy. Yet if we have a constraint for the set of registers allowed here, then
> we give the register
On 05/22/2014 02:33 PM, Kai Tietz wrote:
> * config/i386/i386.c (ix86_expand_call): Enforce for sibcalls
> on memory the use of accumulator-register.
I don't like this at all.
I'm fine with allowing memories that are fully symbolic, e.g.
extern void (*foo)(void);
void f(void) { foo()
Ok, so just the part ok the patch remains to prevent varardic-functions being a
sibling-tail-call remains.
Kai
On 05/26/14 14:32, Kai Tietz wrote:
- Original Message -
On Mon, May 26, 2014 at 03:22:50PM -0400, Kai Tietz wrote:
In any case, I still can't understand how limiting the choices
of the register allocator can improve code rather than making
it worse. If the accumulator is available ther
- Original Message -
> On Mon, May 26, 2014 at 03:22:50PM -0400, Kai Tietz wrote:
> > > In any case, I still can't understand how limiting the choices of the
> > > register allocator can improve code rather than making it worse.
> > > If the accumulator is available there, why doesn't the R
On Mon, May 26, 2014 at 03:22:50PM -0400, Kai Tietz wrote:
> > In any case, I still can't understand how limiting the choices of the
> > register allocator can improve code rather than making it worse.
> > If the accumulator is available there, why doesn't the RA choose it
> > if it is beneficial?
- Original Message -
> On Mon, May 26, 2014 at 02:20:36PM -0400, Kai Tietz wrote:
> > --- i386.c (revision 210936)
> > +++ i386.c (working copy)
> > @@ -5298,6 +5298,12 @@ ix86_function_ok_for_sibcall (tree decl, tree exp)
> >decl_or_type = type;
> > }
> >
> > + /* We need
On Mon, May 26, 2014 at 02:20:36PM -0400, Kai Tietz wrote:
> --- i386.c(revision 210936)
> +++ i386.c(working copy)
> @@ -5298,6 +5298,12 @@ ix86_function_ok_for_sibcall (tree decl, tree exp)
>decl_or_type = type;
> }
>
> + /* We need to reject stdarg-function for x86_64 ABI
Hi,
adjusted patch to make it more bullet-proved and tested it.
2014-05-26 Kai Tietz
* config/i386/i386.c (ix86_expand_call): Enforce for sibcalls
on memory the use of accumulator-register.
(ix86_function_ok_for_sibcall): Reject for x86_64 ABI sibling
calls for
On 05/22/14 15:33, Kai Tietz wrote:
Hello,
This patch avoids for sibling-tail-calls the use of pseudo-register.
Instead it uses for load of memory-address the accumulator-register.
By this we avoid that IRA/LRA need to choose a register. So we
reduce register-pressure.
The accumulator-register
On 05/22/14 16:07, H.J. Lu wrote:
On Thu, May 22, 2014 at 2:33 PM, Kai Tietz wrote:
Hello,
This patch avoids for sibling-tail-calls the use of pseudo-register. Instead
it uses for load of memory-address the accumulator-register. By this we avoid
that IRA/LRA need to choose a register. So
On Thu, May 22, 2014 at 2:33 PM, Kai Tietz wrote:
> Hello,
>
> This patch avoids for sibling-tail-calls the use of pseudo-register. Instead
> it uses for load of memory-address the accumulator-register. By this we
> avoid that IRA/LRA need to choose a register. So we reduce register-pressure.
Hello,
This patch avoids for sibling-tail-calls the use of pseudo-register. Instead
it uses for load of memory-address the accumulator-register. By this we avoid
that IRA/LRA need to choose a register. So we reduce register-pressure.
The accumulator-register is always a valid register on tai
39 matches
Mail list logo
