D.gnu - Inlining problems again
- Artur Skawina (112/112) Apr 04 2014 Built latest gcc4-8-based gdc (fdf0c614) today; immediately ran into thi...
- Johannes Pfau (7/17) Apr 04 2014 Please try to reduce such examples in the future ;-)
- Artur Skawina (9/26) Apr 04 2014 Sorry. Yes, I should have tried harder.
- Artur Skawina (11/28) Apr 04 2014 [...]
- Iain Buclaw (4/27) Apr 04 2014 http://bugzilla.gdcproject.org/show_bug.cgi?id=37
- Iain Buclaw (24/46) Apr 04 2014 The switch -fdump-ipa-inline is your friend here:
- Johannes Pfau (35/41) Apr 05 2014 Root cause is that const(char)[] is a distinct type compared to char[]
- Iain Buclaw (3/18) Apr 05 2014 I've had another thought for a while now that involves not constifying '...
- Johannes Pfau (12/17) Apr 05 2014 But we'd want this to work/inline nevertheless, right?:
- Daniel Murphy (3/13) Apr 05 2014 Can you just strip all const/immutable/etc when the type is passed to th...
- Johannes Pfau (7/26) Apr 05 2014 This would impact debug info which is also emitted by the backend. GCC
- Iain Buclaw (5/31) Apr 05 2014 Right, and not having const applied to the type means that gcc might mis...
- Iain Buclaw (13/45) Apr 05 2014 mapped to C-style 'const'. The 'in' keyword is close, but something oth...
- Iain Buclaw (10/58) Apr 05 2014 See gimple.c: gimple_call_arg_flags for the middle-end detection and
- Johannes Pfau (3/20) Apr 05 2014 Related: http://gcc.gnu.org/ml/gcc/2005-01/msg01656.html
Built latest gcc4-8-based gdc (fdf0c614) today; immediately ran into this:
This program:
import std.array, std.algorithm;
int main(string[] argv) {
auto r = argv.filter!"a.length"().count();
return r&255;
}
compiles to:
00000000004052e0 <pure nothrow property safe bool
std.array.empty!(immutable(char)[]).empty(const(immutable(char)[][]))>:
4052e0: 48 85 ff test %rdi,%rdi
4052e3: 0f 94 c0 sete %al
4052e6: c3 retq
00000000004052f0 <_Dmain>:
4052f0: 41 54 push %r12
4052f2: 49 89 f4 mov %rsi,%r12
4052f5: 55 push %rbp
4052f6: 48 89 fd mov %rdi,%rbp
4052f9: 53 push %rbx
4052fa: 48 83 ec 10 sub $0x10,%rsp
4052fe: 48 89 ef mov %rbp,%rdi
405301: 4c 89 e6 mov %r12,%rsi
405304: 4c 89 e3 mov %r12,%rbx
405307: e8 d4 ff ff ff callq 4052e0 <pure nothrow property
safe bool std.array.empty!(immutable(char)[]).empty(const(immutable(char)[][]))>
40530c: 84 c0 test %al,%al
40530e: 75 18 jne 405328 <_Dmain+0x38>
405310: 48 8d 45 ff lea -0x1(%rbp),%rax
405314: 49 83 c4 10 add $0x10,%r12
405318: 48 83 3b 00 cmpq $0x0,(%rbx)
40531c: 75 0a jne 405328 <_Dmain+0x38>
40531e: 48 89 c5 mov %rax,%rbp
405321: eb db jmp 4052fe <_Dmain+0xe>
405323: 0f 1f 44 00 00 nopl 0x0(%rax,%rax,1)
405328: 48 89 ef mov %rbp,%rdi
40532b: 48 89 de mov %rbx,%rsi
40532e: 45 31 e4 xor %r12d,%r12d
405331: e8 aa ff ff ff callq 4052e0 <pure nothrow property
safe bool std.array.empty!(immutable(char)[]).empty(const(immutable(char)[][]))>
405336: 84 c0 test %al,%al
405338: 75 2a jne 405364 <_Dmain+0x74>
40533a: 48 83 ed 01 sub $0x1,%rbp
40533e: 48 83 c3 10 add $0x10,%rbx
405342: 48 89 ef mov %rbp,%rdi
405345: 48 89 de mov %rbx,%rsi
405348: e8 93 ff ff ff callq 4052e0 <pure nothrow property
safe bool std.array.empty!(immutable(char)[]).empty(const(immutable(char)[][]))>
40534d: 84 c0 test %al,%al
40534f: 74 27 je 405378 <_Dmain+0x88>
405351: 48 89 ef mov %rbp,%rdi
405354: 48 89 de mov %rbx,%rsi
405357: 49 83 c4 01 add $0x1,%r12
40535b: e8 80 ff ff ff callq 4052e0 <pure nothrow property
safe bool std.array.empty!(immutable(char)[]).empty(const(immutable(char)[][]))>
405360: 84 c0 test %al,%al
405362: 74 d6 je 40533a <_Dmain+0x4a>
405364: 48 83 c4 10 add $0x10,%rsp
405368: 41 0f b6 c4 movzbl %r12b,%eax
40536c: 5b pop %rbx
40536d: 5d pop %rbp
40536e: 41 5c pop %r12
405370: c3 retq
405378: 48 83 3b 00 cmpq $0x0,(%rbx)
40537c: 48 8d 55 ff lea -0x1(%rbp),%rdx
405380: 75 cf jne 405351 <_Dmain+0x61>
405382: 48 89 d5 mov %rdx,%rbp
405385: eb b7 jmp 40533e <_Dmain+0x4e>
That trivial std.array.empty() function is for some reason not getting
inlined, leading to the catastrophic result above.
Also checked with an old 4.6 based gdc; that one manages to inline
everything when using LTO (which the newer gdcs can't handle).
If the array is wrapped in a custom range like this:
import std.array, std.algorithm;
auto range(E)(E[] e) property {
static struct AR {
E[] arr;
inout(E) front() inout property { return arr[0]; }
bool empty() const property { return !arr.length; }
void popFront() { arr = arr[1..$]; }
size_t length() property const { return arr.length; }
}
return AR(e);
}
int main(string[] argv) {
auto r = argv.range.filter!"a.length"().count();
return r&255;
}
and compiled with the same (current 4.8-based) compiler and an identical
cmdline, then the result turns into:
00000000004052e0 <_Dmain>:
4052e0: 48 85 ff test %rdi,%rdi
4052e3: 75 03 jne 4052e8 <_Dmain+0x8>
4052e5: 31 c0 xor %eax,%eax
4052e7: c3 retq
4052e8: 48 83 3e 00 cmpq $0x0,(%rsi)
4052ec: 75 10 jne 4052fe <_Dmain+0x1e>
4052ee: 48 83 c6 10 add $0x10,%rsi
4052f2: 48 83 ef 01 sub $0x1,%rdi
4052f6: 74 ed je 4052e5 <_Dmain+0x5>
4052f8: 48 83 3e 00 cmpq $0x0,(%rsi)
4052fc: 74 f0 je 4052ee <_Dmain+0xe>
4052fe: 31 c0 xor %eax,%eax
405300: eb 10 jmp 405312 <_Dmain+0x32>
405302: 66 0f 1f 44 00 00 nopw 0x0(%rax,%rax,1)
405308: 48 83 c6 10 add $0x10,%rsi
40530c: 48 83 3e 00 cmpq $0x0,(%rsi)
405310: 74 04 je 405316 <_Dmain+0x36>
405312: 48 83 c0 01 add $0x1,%rax
405316: 48 83 ef 01 sub $0x1,%rdi
40531a: 75 ec jne 405308 <_Dmain+0x28>
40531c: 0f b6 c0 movzbl %al,%eax
40531f: c3 retq
Which is much more reasonable, and shouldn't require such hacks.
I thought that inlining /templated/ functions across modules already worked;
is this a different problem, and, more importantly, is it fixable?
artur
Apr 04 2014
Am Fri, 04 Apr 2014 11:52:13 +0200 schrieb Artur Skawina <art.08.09 gmail.com>:Built latest gcc4-8-based gdc (fdf0c614) today; immediately ran into this: Which is much more reasonable, and shouldn't require such hacks. I thought that inlining /templated/ functions across modules already worked; is this a different problem, and, more importantly, is it fixable? arturPlease try to reduce such examples in the future ;-) http://goo.gl/SJ0iEq http://goo.gl/ykumCI property bool empty(T)(in T[] a) safe pure nothrow it's the 'in' which causes inlining to fail (const also fails).
Apr 04 2014
Artur Skawina <art.08.09 gmail.com> On 04/04/14 15:14, Johannes Pfau wrote:Am Fri, 04 Apr 2014 11:52:13 +0200 schrieb Artur Skawina <art.08.09 gmail.com>:Built latest gcc4-8-based gdc (fdf0c614) today; immediately ran into this: Which is much more reasonable, and shouldn't require such hacks. I thought that inlining /templated/ functions across modules already worked; is this a different problem, and, more importantly, is it fixable?Please try to reduce such examples in the future ;-)Sorry. Yes, I should have tried harder. [I'm probably subconsciously avoiding going near any phobos code... :)]http://goo.gl/SJ0iEq http://goo.gl/ykumCI property bool empty(T)(in T[] a) safe pure nothrow it's the 'in' which causes inlining to fail (const also fails).Hmm, I remember const-args were a problem in the past, but I had thought this was already fixed. A quick search reveals: http://gdcproject.org/bugzilla/show_bug.cgi?id=37 (-ENOPERM) https://bitbucket.org/goshawk/gdc/issue/300/array-empty-function-does-not-get-inlined (which says WONTFIX, but IIRC Iain fixed) artur
Apr 04 2014
On 04/04/14 15:42, Artur Skawina wrote:On 04/04/14 15:14, Johannes Pfau wrote:Am Fri, 04 Apr 2014 11:52:13 +0200 schrieb Artur Skawina <art.08.09 gmail.com>:Built latest gcc4-8-based gdc (fdf0c614) today; immediately ran into this: Which is much more reasonable, and shouldn't require such hacks. I thought that inlining /templated/ functions across modules already worked; is this a different problem, and, more importantly, is it fixable?[...] Yep, I was remembering correctly: http://forum.dlang.org/post/mailman.33.1325763631.16222.d.gnu puremagic.com [no idea how I managed to forget that report - this bug has exactly the same symptoms] And Iain actually did fix it back then: http://forum.dlang.org/post/mailman.221.1326102842.16222.d.gnu puremagic.com http://forum.dlang.org/post/mailman.233.1326132966.16222.d.gnu puremagic.com But somehow the bug is now back... arturproperty bool empty(T)(in T[] a) safe pure nothrow it's the 'in' which causes inlining to fail (const also fails).Hmm, I remember const-args were a problem in the past, but I had thought this was already fixed. A quick search reveals:
Apr 04 2014
On 4 April 2014 14:42, Artur Skawina <art.08.09 gmail.com> wrote:On 04/04/14 15:14, Johannes Pfau wrote:http://bugzilla.gdcproject.org/show_bug.cgi?id=37 I should figure out what to do with /bugzilla. Maybe just put in a rewrite rule.Am Fri, 04 Apr 2014 11:52:13 +0200 schrieb Artur Skawina <art.08.09 gmail.com>:Built latest gcc4-8-based gdc (fdf0c614) today; immediately ran into this: Which is much more reasonable, and shouldn't require such hacks. I thought that inlining /templated/ functions across modules already worked; is this a different problem, and, more importantly, is it fixable?Please try to reduce such examples in the future ;-)Sorry. Yes, I should have tried harder. [I'm probably subconsciously avoiding going near any phobos code... :)]http://goo.gl/SJ0iEq http://goo.gl/ykumCI property bool empty(T)(in T[] a) safe pure nothrow it's the 'in' which causes inlining to fail (const also fails).Hmm, I remember const-args were a problem in the past, but I had thought this was already fixed. A quick search reveals: http://gdcproject.org/bugzilla/show_bug.cgi?id=37 (-ENOPERM)
Apr 04 2014
On 4 April 2014 14:42, Artur Skawina <art.08.09 gmail.com> wrote:On 04/04/14 15:14, Johannes Pfau wrote:The switch -fdump-ipa-inline is your friend here: --- Inline summary for D main/3 inlinable self time: 22 global time: 0 self size: 14 global size: 0 min size: 0 self stack: 0 global stack: 0 size:4.000000, time:2.778000, predicate:(true) size:3.000000, time:2.000000, predicate:(not inlined) calls: writeln/27 function not considered for inlining loop depth: 0 freq: 389 size: 3 time: 12 callee size:12 stack: 0 empty/26 mismatched arguments loop depth: 0 freq:1000 size: 4 time: 13 callee size: 2 stack: 0 --- Reason for no inline is "mismatched arguments" - which means that the caller and callee disagree on the arguments to pass to the function. The fix is to make sure that caller and callee arguments match when generating the function on both sides. We could prevent this from recurring by putting in an assert in the codegen.Am Fri, 04 Apr 2014 11:52:13 +0200 schrieb Artur Skawina <art.08.09 gmail.com>:Built latest gcc4-8-based gdc (fdf0c614) today; immediately ran into this: Which is much more reasonable, and shouldn't require such hacks. I thought that inlining /templated/ functions across modules already worked; is this a different problem, and, more importantly, is it fixable?Please try to reduce such examples in the future ;-)Sorry. Yes, I should have tried harder. [I'm probably subconsciously avoiding going near any phobos code... :)]http://goo.gl/SJ0iEq http://goo.gl/ykumCI property bool empty(T)(in T[] a) safe pure nothrow it's the 'in' which causes inlining to fail (const also fails).Hmm, I remember const-args were a problem in the past, but I had thought this was already fixed.
Apr 04 2014
Am Sat, 5 Apr 2014 07:37:00 +0100 schrieb Iain Buclaw <ibuclaw gdcproject.org>:Reason for no inline is "mismatched arguments" - which means that the caller and callee disagree on the arguments to pass to the function. The fix is to make sure that caller and callee arguments match when generating the function on both sides. We could prevent this from recurring by putting in an assert in the codegen.Root cause is that const(char)[] is a distinct type compared to char[] so I think we need to make const(char)[] a variant of char[]. We could use build_variant_type_copy and then modify the copy to use the correct basetype. Here's a proof of concept, could you finish this Iain? (We should probably check all types which have a 'next' type if a similar change makes sense for these) --- a/gcc/d/d-ctype.cc +++ b/gcc/d/d-ctype.cc -496,6 +496,21 TypeDArray::toCtype (void) ctype = castMod(0)->toCtype(); ctype = insert_type_modifiers (ctype, mod); } + else if (!next->isNaked()) + { + tree ptrtype = next->toCtype(); + Type *dt = new TypeDArray(next->castMod(0)); + dt = dt->semantic(Loc(NULL, 0), NULL); + ctype = build_variant_type_copy(dt->toCtype()); + + //tree f1 = build_decl (BUILTINS_LOCATION, FIELD_DECL, get_identifier ("ptr"), build_pointer_type (ptrtype)); + //DECL_CONTEXT (f1) = ctype; + //TREE_CHAIN (f1) = NULL_TREE; + //TREE_CHAIN (TYPE_FIELDS (ctype)) = f1; + + TYPE_LANG_SPECIFIC (ctype) = build_d_type_lang_specific (this); + d_keep (ctype); + } else { tree lentype = Type::tsize_t->toCtype();
Apr 05 2014
On 5 Apr 2014 13:45, "Johannes Pfau" <nospam example.com> wrote:Am Sat, 5 Apr 2014 07:37:00 +0100 schrieb Iain Buclaw <ibuclaw gdcproject.org>:I've had another thought for a while now that involves not constifying 'in' parameters, but at the same time not loosing its guarantee.Reason for no inline is "mismatched arguments" - which means that the caller and callee disagree on the arguments to pass to the function. The fix is to make sure that caller and callee arguments match when generating the function on both sides. We could prevent this from recurring by putting in an assert in the codegen.Root cause is that const(char)[] is a distinct type compared to char[] so I think we need to make const(char)[] a variant of char[]. We could use build_variant_type_copy and then modify the copy to use the correct basetype. Here's a proof of concept, could you finish this Iain? (We should probably check all types which have a 'next' type if a similar change makes sense for these)
Apr 05 2014
Am Sat, 5 Apr 2014 15:31:30 +0100 schrieb Iain Buclaw <ibuclaw gdcproject.org>:I've had another thought for a while now that involves not constifying 'in' parameters, but at the same time not loosing its guarantee.But we'd want this to work/inline nevertheless, right?: ------------ void test(const(char)[] a) { } char[] abc; test(abc); ------------ Then we still need to tell GCC that const(char)[] is a variant of char[] or it won't inline.
Apr 05 2014
"Johannes Pfau" wrote in message news:lhp8h4$2j38$1 digitalmars.com...
But we'd want this to work/inline nevertheless, right?:
------------
void test(const(char)[] a)
{
}
char[] abc;
test(abc);
------------
Then we still need to tell GCC that const(char)[] is a variant of
char[] or it won't inline.
Can you just strip all const/immutable/etc when the type is passed to the
backend?
Apr 05 2014
Am Sun, 6 Apr 2014 02:51:28 +1000 schrieb "Daniel Murphy" <yebbliesnospam gmail.com>:"Johannes Pfau" wrote in message news:lhp8h4$2j38$1 digitalmars.com...This would impact debug info which is also emitted by the backend. GCC supports 'variants' of types which means only different qualifiers but the same apart from type qualifiers. We just need to set the variant information correctly (e.g. const(char)[] should be recognized as a variant of char[])But we'd want this to work/inline nevertheless, right?: ------------ void test(const(char)[] a) { } char[] abc; test(abc); ------------ Then we still need to tell GCC that const(char)[] is a variant of char[] or it won't inline.Can you just strip all const/immutable/etc when the type is passed to the backend?
Apr 05 2014
On 5 Apr 2014 19:55, "Johannes Pfau" <nospam example.com> wrote:Am Sun, 6 Apr 2014 02:51:28 +1000 schrieb "Daniel Murphy" <yebbliesnospam gmail.com>:Right, and not having const applied to the type means that gcc might miss an optimisation opportunity. In this case however I think that parameters declared in should not be mapped to C-style 'const'. The 'in' keyword is close, but something other."Johannes Pfau" wrote in message news:lhp8h4$2j38$1 digitalmars.com...This would impact debug info which is also emitted by the backend. GCC supports 'variants' of types which means only different qualifiers but the same apart from type qualifiers. We just need to set the variant information correctly (e.g. const(char)[] should be recognized as a variant of char[])But we'd want this to work/inline nevertheless, right?: ------------ void test(const(char)[] a) { } char[] abc; test(abc); ------------ Then we still need to tell GCC that const(char)[] is a variant of char[] or it won't inline.Can you just strip all const/immutable/etc when the type is passed to the backend?
Apr 05 2014
On 5 Apr 2014 21:31, "Iain Buclaw" <ibuclaw gdcproject.org> wrote:On 5 Apr 2014 19:55, "Johannes Pfau" <nospam example.com> wrote:an optimisation opportunity.Am Sun, 6 Apr 2014 02:51:28 +1000 schrieb "Daniel Murphy" <yebbliesnospam gmail.com>:Right, and not having const applied to the type means that gcc might miss"Johannes Pfau" wrote in message news:lhp8h4$2j38$1 digitalmars.com...This would impact debug info which is also emitted by the backend. GCC supports 'variants' of types which means only different qualifiers but the same apart from type qualifiers. We just need to set the variant information correctly (e.g. const(char)[] should be recognized as a variant of char[])But we'd want this to work/inline nevertheless, right?: ------------ void test(const(char)[] a) { } char[] abc; test(abc); ------------ Then we still need to tell GCC that const(char)[] is a variant of char[] or it won't inline.Can you just strip all const/immutable/etc when the type is passed to the backend?In this case however I think that parameters declared in should not bemapped to C-style 'const'. The 'in' keyword is close, but something other. FORTRAN for instance has intent attributes. --- intent(in) - yes, pass by value, so changes of this are not reflected in outside code. intent(out) - pass somehow by reference, in fact a return argument intent(inout) - pass by reference, normal in/out parameter. --- These are interesting concepts that allows more aggressive optimisation than using C/C++ const/ref type variants. I think gdc should go down this route, but I've never experimented too much round this area.
Apr 05 2014
On 5 April 2014 20:38, Iain Buclaw <ibuclaw gdcproject.org> wrote:On 5 Apr 2014 21:31, "Iain Buclaw" <ibuclaw gdcproject.org> wrote:See gimple.c: gimple_call_arg_flags for the middle-end detection and the implementation is roughly: EAF_UNUSED: Unused EAF_NOCLOBBER: Read-only EAF_NOESCAPE: Not escaping EAF_DIRECT: Only once dereferenced (*p, not **p) The 'in' attribute could be mapped to 'r', meaning read-only and not escaping. Regards Iain.On 5 Apr 2014 19:55, "Johannes Pfau" <nospam example.com> wrote:FORTRAN for instance has intent attributes. --- intent(in) - yes, pass by value, so changes of this are not reflected in outside code. intent(out) - pass somehow by reference, in fact a return argument intent(inout) - pass by reference, normal in/out parameter. --- These are interesting concepts that allows more aggressive optimisation than using C/C++ const/ref type variants. I think gdc should go down this route, but I've never experimented too much round this area.Am Sun, 6 Apr 2014 02:51:28 +1000 schrieb "Daniel Murphy" <yebbliesnospam gmail.com>:Right, and not having const applied to the type means that gcc might miss an optimisation opportunity. In this case however I think that parameters declared in should not be mapped to C-style 'const'. The 'in' keyword is close, but something other."Johannes Pfau" wrote in message news:lhp8h4$2j38$1 digitalmars.com...This would impact debug info which is also emitted by the backend. GCC supports 'variants' of types which means only different qualifiers but the same apart from type qualifiers. We just need to set the variant information correctly (e.g. const(char)[] should be recognized as a variant of char[])But we'd want this to work/inline nevertheless, right?: ------------ void test(const(char)[] a) { } char[] abc; test(abc); ------------ Then we still need to tell GCC that const(char)[] is a variant of char[] or it won't inline.Can you just strip all const/immutable/etc when the type is passed to the backend?
Apr 05 2014
Am Sat, 5 Apr 2014 15:31:30 +0100 schrieb Iain Buclaw <ibuclaw gdcproject.org>:On 5 Apr 2014 13:45, "Johannes Pfau" <nospam example.com> wrote:Related: http://gcc.gnu.org/ml/gcc/2005-01/msg01656.htmlRoot cause is that const(char)[] is a distinct type compared to char[] so I think we need to make const(char)[] a variant of char[]. We could use build_variant_type_copy and then modify the copy to use the correct basetype. Here's a proof of concept, could you finish this Iain? (We should probably check all types which have a 'next' type if a similar change makes sense for these)I've had another thought for a while now that involves not constifying 'in' parameters, but at the same time not loosing its guarantee.
Apr 05 2014