Re: Dealing with paradoxical subregs of memory?

2017-01-26 Thread Dominik Vogt
On Wed, Jan 25, 2017 at 04:45:23PM -0600, Segher Boessenkool wrote:
> On Wed, Jan 25, 2017 at 06:36:04PM +0100, Dominik Vogt wrote:
> > On the other hand, Combine
> > does not know that they are "outlawed" and happily generates
> > them.
> 
> combine should not generate things that can never match.  Of course it
> sometimes does.  This should be improved; please open a PR.

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=79238

Ciao

Dominik ^_^  ^_^

-- 

Dominik Vogt
IBM Germany



Dealing with paradoxical subregs of memory?

2017-01-25 Thread Dominik Vogt
What's the matter with paradoxical subregs of memory?  So, as far
as I understand, Recog refuses to match them unless they are
explicitly expressed as a pattern.  On the other hand, Combine
does not know that they are "outlawed" and happily generates
them.

This happens sometimes with some new patterns for s390x for the
RISBG instruction, which does complex operations on registers.  As
it's beneficial to allow memory operands before reload,
paradoxical subregs of memory are sometimes combined from such
patterns, for example

  (set (reg:DI 68)
   (and:DI (subreg:DI (lshiftrt:SI (reg:SI 67 [ *f_5(D) ])
  (const_int 8 [0x8])) 0)
   (const_int 16777215 [0xff])))

+

  (set (reg/i:DI 2 %r2)
(and:DI (reg:DI 68)
(const_int 1 [0x1])))


=

(set (reg/i:DI 2 %r2)
 (and:DI (subreg:DI (mem:QI (plus:DI (reg:DI 2 %r2 [ f ])
  ^
(const_int 2 [0x2])) [1 *f_5(D)+2 S1 A16]) 0)
 (const_int 1 [0x1])))

Which doesn't match because of the paradoxical subreg of memory (I
think).

Wouldn't it be better to allow that inside combine, then ann a
pass right after it that forces a reaload into a pseudo for such
memory references (for the targets that don't want them)?  (Not
sure if theat would result in a noticeable gain or not.)

Ciao

Dominik ^_^  ^_^

-- 

Dominik Vogt
IBM Germany



Re: Mysterious decision in combine

2016-03-21 Thread Dominik Vogt
On Thu, Mar 17, 2016 at 01:22:04PM -0700, Richard Henderson wrote:
> On 03/16/2016 11:35 PM, Dominik Vogt wrote:
> > How does combine get this idea (it's the only match in the
> > function)?
> > 
> >   Trying 7 -> 12:
> >   Successfully matched this instruction:
> >   (set (reg/i:DI 2 %r2)
> >   (and:DI (subreg:DI (reg:SI 64) 0)
> >   (const_int 4294901775 [0x000f])))
> >   allowing combination of insns 7 and 12
> 
> >From the recorded nonzero_bits.

Understood so far.  Now combine tries to combine

  (parallel [
(set (reg:SI 64)
(and:SI (mem:SI (reg:DI 2 %r2 [ a ]) [1 *a_2(D)+0 S4 A32])
(const_int -65521 [0x000f])))
(clobber (reg:CC 33 %cc))
])

and

  (set (reg:DI 65)
(zero_extend:DI (reg:SI 64)))

Why does it drop the "parallel" and "clobber" in the combination;
is there a way to force combine to keep that?

  Trying 6 -> 7:
  Failed to match this instruction:
  (set (reg:DI 65)
  (and:DI (subreg:DI (mem:SI (reg:DI 2 %r2 [ a ]) [1 *a_2(D)+0 S4 A32]) 0)
  (const_int 4294901775 [0x000f])))

(Because all "and" instructions on s390 clobber the CC, this
pattern does not match anything without the clobber.)

Ciao

Dominik ^_^  ^_^

-- 

Dominik Vogt
IBM Germany



Mysterious decision in combine

2016-03-19 Thread Dominik Vogt
Well, at least what combine does here is a mystery to me (s390x
with -O3 in case it matters).

Rtl before combine:

-- snip --
(insn 6 3 7 2 (parallel [
(set (reg:SI 64)
(and:SI (mem:SI (reg/v/f:DI 63 [ a ]) [1 *a_2(D)+0 S4 A32])
(const_int -65521 [0x000f])))
(clobber (reg:CC 33 %cc))
]) andc-immediate.c:21 1481 {*andsi3_zarch}
 (expr_list:REG_DEAD (reg/v/f:DI 63 [ a ])
(expr_list:REG_UNUSED (reg:CC 33 %cc)
(nil
(insn 7 6 12 2 (set (reg:DI 65)
(zero_extend:DI (reg:SI 64))) andc-immediate.c:21 1207 
{*zero_extendsidi2}
 (expr_list:REG_DEAD (reg:SI 64)
(nil)))
(insn 12 7 13 2 (set (reg/i:DI 2 %r2)
(reg:DI 65)) andc-immediate.c:22 1073 {*movdi_64}
 (expr_list:REG_DEAD (reg:DI 65)
(nil)))
-- snip --

How does combine get this idea (it's the only match in the
function)?

  Trying 7 -> 12:
  Successfully matched this instruction:
  (set (reg/i:DI 2 %r2)
  (and:DI (subreg:DI (reg:SI 64) 0)
  (const_int 4294901775 [0x000f])))
  allowing combination of insns 7 and 12

=>

-- snip --
(insn 6 3 7 2 (parallel [
(set (reg:SI 64)
(and:SI (mem:SI (reg:DI 2 %r2 [ a ]) [1 *a_2(D)+0 S4 A32])
(const_int -65521 [0x000f])))
(clobber (reg:CC 33 %cc))
]) andc-immediate.c:21 1481 {*andsi3_zarch}
 (expr_list:REG_DEAD (reg:DI 2 %r2 [ a ])
(expr_list:REG_UNUSED (reg:CC 33 %cc)
(nil
(insn 12 7 13 2 (parallel [
(set (reg/i:DI 2 %r2)
(and:DI (subreg:DI (reg:SI 64) 0)
 ^^^
(const_int 4294901775 [0x000f])))
   ^^
(clobber (reg:CC 33 %cc))
]) andc-immediate.c:22 1474 {*anddi3}
 (expr_list:REG_UNUSED (reg:CC 33 %cc)
(expr_list:REG_DEAD (reg:SI 64)
(nil
-- snip --

It combines "zero extend" + "copy to hardreg" into an "and with
0x000f".  That is the correct result for combining insn 6 + 7
+ 12, however.  (Eventually the two "and"s with constant values
are not merged into a single "and" with a single constant.)

(dumps attached)

Ciao

Dominik ^_^  ^_^

-- 

Dominik Vogt
IBM Germany

;; Function andc_32_pv (andc_32_pv, funcdef_no=0, decl_uid=1973, cgraph_uid=0, 
symbol_order=0)

starting the processing of deferred insns
ending the processing of deferred insns
df_analyze called
df_worklist_dataflow_doublequeue:n_basic_blocks 3 n_edges 2 count 3 (1)


andc_32_pv

Dataflow summary:
;;  invalidated by call  0 [%r0] 1 [%r1] 2 [%r2] 3 [%r3] 4 [%r4] 5 
[%r5] 16 [%f0] 17 [%f2] 18 [%f4] 19 [%f6] 20 [%f1] 21 [%f3] 22 [%f5] 23 [%f7] 
33 [%cc] 35 [%rp] 38 [%v16] 39 [%v18] 40 [%v20] 41 [%v22] 42 [%v17] 43 [%v19] 
44 [%v21] 45 [%v23] 46 [%v24] 47 [%v26] 48 [%v28] 49 [%v30] 50 [%v25] 51 [%v27] 
52 [%v29] 53 [%v31]
;;  hardware regs used   15 [%r15] 32 [%ap] 34 [%fp]
;;  regular block artificial uses11 [%r11] 15 [%r15] 32 [%ap] 34 [%fp]
;;  eh block artificial uses 11 [%r11] 15 [%r15] 32 [%ap] 34 [%fp]
;;  entry block defs 0 [%r0] 2 [%r2] 3 [%r3] 4 [%r4] 5 [%r5] 6 [%r6] 11 
[%r11] 14 [%r14] 15 [%r15] 16 [%f0] 17 [%f2] 18 [%f4] 19 [%f6] 32 [%ap] 34 [%fp]
;;  exit block uses  2 [%r2] 11 [%r11] 14 [%r14] 15 [%r15] 34 [%fp]
;;  regs ever live   2 [%r2] 33 [%cc]
;;  ref usage   r0={1d} r2={2d,3u} r3={1d} r4={1d} r5={1d} r6={1d} r11={1d,2u} 
r14={1d,1u} r15={1d,2u} r16={1d} r17={1d} r18={1d} r19={1d} r32={1d,1u} 
r33={1d} r34={1d,2u} r63={1d,1u} r64={1d,1u} r65={1d,1u} 
;;total ref usage 34{20d,14u,0e} in 5{5 regular + 0 call} insns.
;; Reaching defs:
;;  sparse invalidated  
;;  dense invalidated   0, 1, 2, 3, 4, 5, 10, 11, 12, 13, 15
;;  reg->defs[] map:0[0,0] 2[1,2] 3[3,3] 4[4,4] 5[5,5] 6[6,6] 11[7,7] 
14[8,8] 15[9,9] 16[10,10] 17[11,11] 18[12,12] 19[13,13] 32[14,14] 33[15,15] 
34[16,16] 63[17,17] 64[18,18] 65[19,19] 

( )->[0]->( 2 )
;; bb 0 artificial_defs: { d0(0){ }d2(2){ }d3(3){ }d4(4){ }d5(5){ }d6(6){ 
}d7(11){ }d8(14){ }d9(15){ }d10(16){ }d11(17){ }d12(18){ }d13(19){ }d14(32){ 
}d16(34){ }}
;; bb 0 artificial_uses: { }
;; lr  in   
;; lr  use  
;; lr  def   0 [%r0] 2 [%r2] 3 [%r3] 4 [%r4] 5 [%r5] 6 [%r6] 11 [%r11] 14 
[%r14] 15 [%r15] 16 [%f0] 17 [%f2] 18 [%f4] 19 [%f6] 32 [%ap] 34 [%fp]
;; live  in 
;; live  gen 0 [%r0] 2 [%r2] 3 [%r3] 4 [%r4] 5 [%r5] 6 [%r6] 11 [%r11] 14 
[%r14] 15 [%r15] 16 [%f0] 17 [%f2] 18 [%f4] 19 [%f6] 32 [%ap] 34 [%fp]
;; live  kill   
;; rd  in   (0) 
;; rd  gen  (15) 
0[0],2[2],3[3],4[4],5[5],6[6],11[7],14[8],15[9],16[10],17[11],18[12],19[13],32[14],34[16]
;; rd  kill (16) 
0[0],2[1,2],3[3],4[4],5[5],6[6],11[7],14[8],15[9],16[10],17[11],18[12],19[13],32[14],34[16]
;;  UD chains for artificial uses at top
;; lr  out   2 [%r2] 11 [%r11] 14 [%r14] 1

Documentation issue: gccint: define_split "not" allowed to create pseudos

2016-03-14 Thread Dominik Vogt
(This is a copy of https://gcc.gnu.org/bugzilla/show_bug.cgi?id=70078)

I'd like to clean up this documentation issue, but need some help:

Dominik Vogt 2016-03-04 11:05:16 UTC
> The section "Defining How to Split Instructions" in the gccint
> manual claims
>
>   The preparation-statements are similar to those statements that are
>   specified for define_expand.
>   ...
>   Unlike those in define_expand, however, these statements must not
>   generate any new pseudo-registers.  Once reload has completed, they
>   also must not allocate any space in the stack frame.
>
> Splitters seem to be allowed to generate new pseudos under
> certain circumstances (some splitters call can_create_psudo_p()).
> So, is this correct instead?
>   ...
>   Unlike those in define_expand, however, once reload has completed
>   these statements must neither generate any new pseudo-registers nor
>   allocate any space in the stack frame.  This can be checked by calling
>   can_create_pseudo_p.

Comment 1 Dominik Vogt 2016-03-04 11:45:00 UTC
> Hijacking this bug report for more unclear documentation in that
> section; proposed changes in marked with <...>.
> 
> Apart from the bad grammar, the meaning of this sentence is a
> mystery:
> 
>   Splitting of jump instruction into sequence that over by another jump 
>   instruction is always valid, as compiler expect identical behavior of
>   new jump.
> 
> =>
> 
>   Splitting of jump instruction into  sequence that 
>   another jump instruction is always valid, as  compiler
>   expect .
> 
> Anybody able to fill in the gaps?

Comment 2 Dominik Vogt 2016-03-04 11:50:36 UTC
> (I'll make a patch with these and some more corrections once it's
> lear how the wording should be.)

Ciao

Dominik ^_^  ^_^

-- 

Dominik Vogt
IBM Germany



Re: Bootstrapping is currently broken

2016-03-08 Thread Dominik Vogt
On Mon, Mar 07, 2016 at 03:41:37PM +0100, Dominik Vogt wrote:
> On Mon, Mar 07, 2016 at 03:18:34PM +0100, Richard Biener wrote:
> > On Mon, Mar 7, 2016 at 3:12 PM, Dominik Vogt  
> > wrote:
> > > On Mon, Mar 07, 2016 at 03:00:03PM +0100, Richard Biener wrote:
> > >> On Mon, Mar 7, 2016 at 2:12 PM, Dominik Vogt  
> > >> wrote:
> > >> > A recent patch has broken bootstrapping (s390x) in stage3.  The
> > >> > failure creeped into trunk between friday and today:
> > >> >
> > >> > -- snip --
> > >> > g++ -std=gnu++98   -g -O2 -DIN_GCC -fno-exceptions -fno-rtti 
> > >> > -fasynchronous-unwind-tables -W -Wall -Wno-narrowing -Wwrite-strings 
> > >> > -Wcast-qual -Wmissing-format-attribute -Woverloaded-virtual -pedantic 
> > >> > -Wno-long-long -Wno-variadic-macros -Wno-overlength-strings -Werror   
> > >> > -DHAVE_CONFIG_H -DGENERATOR_FILE -fno-PIE  -no-pie -o build/gencondmd \
> > >> > build/gencondmd.o .././libiberty/libiberty.a
> > >> > g++: error: unrecognized command line option ‘-no-pie’
> > >> > -- snip --
> > >> >
> > >> > (The compiler in PATH is "gcc (GCC) 4.8.5 20150623 (Red Hat
> > >> > 4.8.5-1)").
> > >>
> > >> Bootstrap should use the built compiler from stage2 in stage3, not
> > >> sure how you get the system compiler used there.
> > >
> > > I guess some configure script failed to notice that g++ is not
> > > being built, and a recent change introduced an option that the
> > > installed compiler doesn't have?  Probably configure should throw
> > > an error if bootstrapping is enabled but the c++ language is not
> > > enabled?

Unreproducable at the moment because I cannot remember what steps
are required to trigger it.  Sometimes in the past I managed to
enable bootstrapping and then the c++ compiler was missing, but it
didn't use the system compiler.  I'll keep my eyes open.

Ciao

Dominik ^_^  ^_^

-- 

Dominik Vogt
IBM Germany



Re: Bootstrapping is currently broken

2016-03-07 Thread Dominik Vogt
On Mon, Mar 07, 2016 at 03:18:34PM +0100, Richard Biener wrote:
> On Mon, Mar 7, 2016 at 3:12 PM, Dominik Vogt  wrote:
> > On Mon, Mar 07, 2016 at 03:00:03PM +0100, Richard Biener wrote:
> >> On Mon, Mar 7, 2016 at 2:12 PM, Dominik Vogt  
> >> wrote:
> >> > A recent patch has broken bootstrapping (s390x) in stage3.  The
> >> > failure creeped into trunk between friday and today:
> >> >
> >> > -- snip --
> >> > g++ -std=gnu++98   -g -O2 -DIN_GCC -fno-exceptions -fno-rtti 
> >> > -fasynchronous-unwind-tables -W -Wall -Wno-narrowing -Wwrite-strings 
> >> > -Wcast-qual -Wmissing-format-attribute -Woverloaded-virtual -pedantic 
> >> > -Wno-long-long -Wno-variadic-macros -Wno-overlength-strings -Werror   
> >> > -DHAVE_CONFIG_H -DGENERATOR_FILE -fno-PIE  -no-pie -o build/gencondmd \
> >> > build/gencondmd.o .././libiberty/libiberty.a
> >> > g++: error: unrecognized command line option ‘-no-pie’
> >> > -- snip --
> >> >
> >> > (The compiler in PATH is "gcc (GCC) 4.8.5 20150623 (Red Hat
> >> > 4.8.5-1)").
> >>
> >> Bootstrap should use the built compiler from stage2 in stage3, not
> >> sure how you get the system compiler used there.
> >
> > I guess some configure script failed to notice that g++ is not
> > being built, and a recent change introduced an option that the
> > installed compiler doesn't have?  Probably configure should throw
> > an error if bootstrapping is enabled but the c++ language is not
> > enabled?
> 
> It is always enabled, are you sure it's not a pilot error?

Sorry, I don't understand the term "pilot error".  I'm currently
rerunning the build to make sure I'm looking at the right compile
log.  There are so many test builds on the disk that I'm not quite
sure I've looked at the right build log.  (The bootstrap error is
real though.).

Ciao

Dominik ^_^  ^_^

-- 

Dominik Vogt
IBM Germany



Re: Bootstrapping is currently broken

2016-03-07 Thread Dominik Vogt
On Mon, Mar 07, 2016 at 03:00:03PM +0100, Richard Biener wrote:
> On Mon, Mar 7, 2016 at 2:12 PM, Dominik Vogt  wrote:
> > A recent patch has broken bootstrapping (s390x) in stage3.  The
> > failure creeped into trunk between friday and today:
> >
> > -- snip --
> > g++ -std=gnu++98   -g -O2 -DIN_GCC -fno-exceptions -fno-rtti 
> > -fasynchronous-unwind-tables -W -Wall -Wno-narrowing -Wwrite-strings 
> > -Wcast-qual -Wmissing-format-attribute -Woverloaded-virtual -pedantic 
> > -Wno-long-long -Wno-variadic-macros -Wno-overlength-strings -Werror   
> > -DHAVE_CONFIG_H -DGENERATOR_FILE -fno-PIE  -no-pie -o build/gencondmd \
> > build/gencondmd.o .././libiberty/libiberty.a
> > g++: error: unrecognized command line option ‘-no-pie’
> > -- snip --
> >
> > (The compiler in PATH is "gcc (GCC) 4.8.5 20150623 (Red Hat
> > 4.8.5-1)").
> 
> Bootstrap should use the built compiler from stage2 in stage3, not
> sure how you get the system compiler used there.

I guess some configure script failed to notice that g++ is not
being built, and a recent change introduced an option that the
installed compiler doesn't have?  Probably configure should throw
an error if bootstrapping is enabled but the c++ language is not
enabled?

Ciao

Dominik ^_^  ^_^

-- 

Dominik Vogt
IBM Germany



Bootstrapping is currently broken

2016-03-07 Thread Dominik Vogt
A recent patch has broken bootstrapping (s390x) in stage3.  The
failure creeped into trunk between friday and today:

-- snip --
g++ -std=gnu++98   -g -O2 -DIN_GCC -fno-exceptions -fno-rtti 
-fasynchronous-unwind-tables -W -Wall -Wno-narrowing -Wwrite-strings 
-Wcast-qual -Wmissing-format-attribute -Woverloaded-virtual -pedantic 
-Wno-long-long -Wno-variadic-macros -Wno-overlength-strings -Werror   
-DHAVE_CONFIG_H -DGENERATOR_FILE -fno-PIE  -no-pie -o build/gencondmd \
build/gencondmd.o .././libiberty/libiberty.a
g++: error: unrecognized command line option ‘-no-pie’
-- snip --

(The compiler in PATH is "gcc (GCC) 4.8.5 20150623 (Red Hat
4.8.5-1)").

Ciao

Dominik ^_^  ^_^

-- 

Dominik Vogt
IBM Germany



Re: Strange C++ function pointer test

2015-12-31 Thread Dominik Vogt
On Thu, Dec 31, 2015 at 12:42:56PM +, Jonathan Wakely wrote:
> On 31 December 2015 at 11:54, Dominik Vogt wrote:
> > Is there a requirement for a certain minimum Glibc version for
> > this to work?
> 
> It doesn't work with any glibc, because it doesn't declare the C++ overloads.

All right, so, the situation is:

 * The test includes just stdlib.h and math.h.
 * The test expects the float variant of abs to be available.
 * It tests the C++ compiler on C code, so it *does not* use any
   C++ specific code (namespace or C++ headers).
 * The C++-11 standard does *not* require that any overloaded
   variants of abs() are available when inclding math.h, right?

==>

The assumption that the test should compile without error is
bogus?  The test seems to assume that including math.h with the
C++ compiler is the same as including cmath.

> Libstdc++ has an include/c_compatibility/math.h header that would
> include  (which declares the C++ overloads) and then pull them
> into the global namespace, but that isn't used on GNU/Linux, and would
> create other problems.
> 
> This is already in Bugzilla: 
> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=60401

Yet this bug report wants stdlib.h (or math.h) included from C++
to add the non-int signatures to the abs() function?

Ciao

Dominik ^_^  ^_^

-- 

Dominik Vogt
IBM Germany



Re: Strange C++ function pointer test

2015-12-31 Thread Dominik Vogt
On Thu, Dec 31, 2015 at 12:45:06PM +0100, Marc Glisse wrote:
> On Thu, 31 Dec 2015, Dominik Vogt wrote:
> 
> >The minimal failing program is
> >
> >-- abs.C --
> >#include 
> >static float (*p1_)(float) = abs;
> >-- abs.C --
> 
> This is allowed to fail. If you include math.h (in addition or
> instead of stdlib.h), it has to work (gcc bug if it doesn't).
> 
> See also
> http://www.open-std.org/jtc1/sc22/wg21/docs/lwg-active.html#2294

Understood.  So, this should work in any case but doesn't

-- abs.C --
#include 
#include 
static float (*p1_)(float) = abs;
-- abs.C --

$ g++ -std=c++11 abs.C -Wsystem-headers
abs.C:3:30: error: invalid conversion from ‘int (*)(int) throw ()’ to ‘float 
(*)(float)’ [-fpermissive]
 static float (*p1_)(float) = abs;
^~~

Is there a requirement for a certain minimum Glibc version for
this to work?

Ciao

Dominik ^_^  ^_^

-- 

Dominik Vogt
IBM Germany



Re: Strange C++ function pointer test

2015-12-31 Thread Dominik Vogt
On Thu, Dec 31, 2015 at 10:11:55AM +, Jonathan Wakely wrote:
> On 31 December 2015 at 09:57, Marc Glisse wrote:
> > On Thu, 31 Dec 2015, Dominik Vogt wrote:
> >
> >> This snippet ist from the Plumhall 2014 xvs test suite:
> >>
> >>  #if CXX03 || CXX11 || CXX14
> >>  static float (*p1_)(float) = abs;
> >>  ...
> >>  checkthat(__LINE__, p1_ != 0);
> >>  #endif
> >>
> >> (With the testsuite specific macros doing the obvious).  abs() is
> >> declared as:
> >>
> >>  int abs(int j)
> >>
> >> Am I missing some odd C++ feature or is that part of the test just
> >> plain wrong?  I don't know where to look in the C++ standard; is
> 
> Try searching the library clauses (17 to 30) for "abs". You'll find
> the answer in 26.8 [c.math].
> 
> >> this supposed to compile (with or without a warning?) or generate
> >> an error or is it just undefined?
> >>
> >>  error: invalid conversion from ‘int (*)(int) throw ()’ to ‘float
> >> (*)(float)’ [-fpermissive]
> >>
> >> (Of course even with -fpermissive this won't work because (at
> >> least on my platform) ints are passed in different registers than
> >> floats.)
> >
> >
> > There are other overloads of 'abs' declared in math.h / cmath (only in
> > namespace std in the second case, and there are bugs (or standard issues)
> > about having them in the global namespace for the first one).
> 
> That's not quite accurate, C++11 was altered slightly to reflect reality.
 
>  is required to declare std::abs and it's unspecified whether
> it also declares it as ::abs.
> 
>  is required to declare ::abs and it's unspecified whether it
> also declares it as std::abs.

(*abs should be in  or ).

I see.  abs and ::abs are the same in this case, and std::abs
refers to the "double" variant, so that does not compile either.

The minimal failing program is

-- abs.C --
#include  
static float (*p1_)(float) = abs;
-- abs.C --

G++-6.0.0, Glibc-2.20 on S390x, compiled with

  $ g++ -std=c++11 abs.C

The test program from the xvs suite includes stdlib.h and math.h,
but not cstdlib, cmath (or ctgmath).  It's not clear to me which
headers g++ includes automatically, but this seems to be correct
behaviour, or is it not?  Does it depend on the Glibc version?

Ciao

Dominik ^_^  ^_^

-- 

Dominik Vogt
IBM Germany



Strange C++ function pointer test

2015-12-31 Thread Dominik Vogt
This snippet ist from the Plumhall 2014 xvs test suite:

  #if CXX03 || CXX11 || CXX14 
  static float (*p1_)(float) = abs; 
  ...
  checkthat(__LINE__, p1_ != 0);
  #endif

(With the testsuite specific macros doing the obvious).  abs() is
declared as:

  int abs(int j)

Am I missing some odd C++ feature or is that part of the test just
plain wrong?  I don't know where to look in the C++ standard; is
this supposed to compile (with or without a warning?) or generate
an error or is it just undefined?

  error: invalid conversion from ‘int (*)(int) throw ()’ to ‘float (*)(float)’ 
[-fpermissive]

(Of course even with -fpermissive this won't work because (at
least on my platform) ints are passed in different registers than
floats.)

Ciao

Dominik ^_^  ^_^

-- 

Dominik Vogt
IBM Germany



Re: Help with detection of an invariant

2015-12-07 Thread Dominik Vogt
On Mon, Dec 07, 2015 at 11:48:10AM -0800, Andrew Pinski wrote:
> On Mon, Dec 7, 2015 at 6:44 AM, Dominik Vogt  wrote:
> > On S/390 the test case gcc.dg/loop-9.c currently fails:
> >
> >   void f (double *a)
> >   {
> > int i;
> > for (i = 0; i < 100; i++)
> >   a[i] = 18.4242;
> >   }
> >
> > It seems to expect that moving 18.4242 to a register is moved out
> > of the loop, but on S/390 it isn't.  It turns out that
> > move_invariant_reg() is never called from move_invariants()
> > because the invariants vector is empty.  Now,
> > find_invariant_insn() checks the insn for invariants using
> > check_dependencies().
> >
> >   (insn 29 28 30 3 (set (mem:DF (reg:DI 81 [ ivtmp.8 ]) [0 MEM[base: _15, 
> > offset: 0B]+0 S8 A64])
> >   (const_double:DF 
> > 1.842419990222931955941021442413330078125e+1 
> > [0x0.9364c2f837b4ap+5])) .../loop-9.c:9 918 {*movdf_64}
> >(nil))
> >
> > check_dependencies() comes across reg 81 first, decides that is
> > not an invariant and returns false so that find_invariant_insn()
> > never even looks at the constant.
> >
> > Actually, the constant should be moved (from the literal pool) to
> > a floating point register (and actually is in the assembly
> > output), and that move could be moved out of the loop (it's not).
> > Where should I look for the root cause?
> 
> 
> Hmm,
>   I want to say the predicates on movdf_64 are too lose allowing the
> above when it should not.   That is movdf_64 should have pushed the
> load of the fp constant into its psedu-register and used that to do
> the storing.

Thanks!  It turns out that for historical S/390[x] moves such
constants to the literal pool only after the loop-invariants pass.
That is because on old cpus it was more efficient to do a direct
memory to memory move with the MVC instruction (i.e. moving the
constant to a register warly is harmful on old cpus).

Ciao

Dominik ^_^  ^_^

-- 

Dominik Vogt
IBM Germany



Help with detection of an invariant

2015-12-07 Thread Dominik Vogt
On S/390 the test case gcc.dg/loop-9.c currently fails:

  void f (double *a) 
  { 
int i; 
for (i = 0; i < 100; i++) 
  a[i] = 18.4242; 
  }

It seems to expect that moving 18.4242 to a register is moved out
of the loop, but on S/390 it isn't.  It turns out that
move_invariant_reg() is never called from move_invariants()
because the invariants vector is empty.  Now,
find_invariant_insn() checks the insn for invariants using
check_dependencies().

  (insn 29 28 30 3 (set (mem:DF (reg:DI 81 [ ivtmp.8 ]) [0 MEM[base: _15, 
offset: 0B]+0 S8 A64])
  (const_double:DF 1.842419990222931955941021442413330078125e+1 
[0x0.9364c2f837b4ap+5])) .../loop-9.c:9 918 {*movdf_64}
   (nil))

check_dependencies() comes across reg 81 first, decides that is
not an invariant and returns false so that find_invariant_insn()
never even looks at the constant.

Actually, the constant should be moved (from the literal pool) to
a floating point register (and actually is in the assembly
output), and that move could be moved out of the loop (it's not).
Where should I look for the root cause?

Ciao

Dominik ^_^  ^_^

-- 

Dominik Vogt
IBM Germany



Re: [RFC] Cse reducing performance of register allocation with -O2

2015-11-23 Thread Dominik Vogt
On Tue, Oct 13, 2015 at 11:06:48AM -0600, Jeff Law wrote:
> On 10/13/2015 07:12 AM, Dominik Vogt wrote:
> >In some cases, the work of the cse1 pass is counterproductive, as
> >we noticed on s390x.  The effect described below is present since
> >at least 4.8.0.  Note that this may not become manifest in a
> >performance issue problem on all platforms.  Also note that -O1
> >does not show this behaviour because the responsible code is only
> >executed with -O2 or higher.
> >
> >The core of the problem is the was cse1 sometimes handles function
> >parameters.  Roughly, the observed situation is
> >
> >Before cse1
> >
> >   start of function
> >   set pseudoreg Rp to the first argument from hardreg R2
> >   (some code that uses Rp)
> >   set R2 to Rp
> >
> >After cse1:
> >
> >   start of function
> >   set pseudoreg Rp to the first argument from hardreg R2
> >   (some code that uses Rp)  <--- The use of Rp is still present
> >   set R2 to R2  <--- cse1 has replaced Rp with R2
> >
> >After that, the set pattern is removed completely, and now we have
> >both, Rp and R2 live in the drafted code snippet.  Because R2 ist
> >still supposed to be live later on, the ira pass chooses a
> >different hard register (R1) for Rp, and code to copy R1 back to
> >R2 is added later.  (See further down for Rtl and assembly code.)
...
> >So, I've made an experimental hack (see attachment) and treid
> >that.  In a larger test suite, register copies could be saved in
> >quite some places (including the test program below), but in other
> >places new register copies were introduced, resulting in about
> >twice as much "issues" as without the patch.
> >
> >Maybe the patch is just too coarse.  In general I'd assume that
> >the register allocator does a better job of assigning hard
> >registers to pseudo registers.  Is it possible to better describe
> >when cse1 should keep its hands off pseudo registers?
> We don't really have a way to describe this.
> 
> I know Vlad looked at problems in this space -- essentially knowing
> when two registers had the same value in the allocators/reload and
> exploiting that information.
> 
> My recollection was it didn't help in any measurable way -- I think
> he discussed it during one of the old GCC summit conferences.  That
> was also in the reload era.
> 
> Ultimately this feels like all the issues around coalescing and
> copy-propagation. With that in mind, if we had lifetime & conflict
> information, then we'd be able to query that and perhaps be able to
> make different choices.

I've spent some more time to try out the naive approach of
detecting this situation in cse_insn().

1. In cse_insn()

  IF current "set" is "set Hardreg H := Pseudoreg P"
  AND  P is generated as a copy of C further up in the extended BB
  AND  P and H still contain the same value
  AND  Cse considers to replace the set with "set H := H"
  AND  P is still live at the end of the EBB
   (In the test program this prevents that *all uses of P are
   replaced by H.)
  THEN do not replace

  => Testing this with the Spec 2006 suite on S390 results in a
  small gain in some cases, a small loss im lots of cases, and a
  substantial win in two cases and a substantial loss in one.  On
  average there is a small win.  I've not tested that on x86, but
  assuming that x86 does not suffer from the original problem I
  expect to see mostly losses.

  This patch requires that a per-register bitmap is created for
  each EBB to record which pseude registers have been generated
  inside the EBB.

2. 

  IF current "set" is "set Hardreg H := Pseudoreg P"
  AND  P is generated as a copy of C further up in the extended BB
  AND  P and H still contain the same value
  AND  Cse considers to replace the set with "set H := H"
  AND  P is still live at the end of the EBB
  AND  P is used between generation and the current instruction.
  THEN do not replace

  => Has fewer win and fewer loss situations and is only slightly
 better on average than (1).  No real improvement.

  This patch requires scanning every insn in cse_insn() for all
  uses of all pseudo registers.  At the moment there is no
  function in rtlanal.c to do this in one call, so I've just
  scanned for each one individually, causing a dramatic increase
  of compile time (* 2 or even more).

So, my conclusion is that the attempt to fix this by patching
cse_insn() is more or less futile.  Replacing the pseudo register
with thte hard register early is actually often a *good* thing,
and to determine whether it's good or bad the code in cse_insn()
would have to correctly guess what later passes do.

Ciao

Dominik ^_^  ^_^

-- 

Dominik Vogt
IBM Germany




Re: [RFC] Cse reducing performance of register allocation with -O2

2015-10-22 Thread Dominik Vogt
On Tue, Oct 13, 2015 at 05:03:36PM -0400, Vladimir Makarov wrote:
[snip]
> I checked my article
> 
> ftp://ftp.uvsq.fr/pub/gcc/summit/2004/Fighting%20Register%20Pressure.pdf
> 
> and GVN gave mostly 0.2% on eon only.  The current environment is
> quite different (IRA, LRA) so the results might be different too.
> 
> Also as I remember I implemented GVN only for pseudos.
> 
> LRA also checks values too but again only for reload and original pseudos.

Do you still have the branches you've tested back then?  I'd
really like to try how this patch affects other targets (big
endian?).  Gcc seems to do a better job optimising code for x86 in
some complicated situations, so the extra logic might pay off more
on other targets.

Ciao

Dominik ^_^  ^_^

-- 

Dominik Vogt
IBM Germany



[RFC] Cse reducing performance of register allocation with -O2

2015-10-13 Thread Dominik Vogt
dfinit ---

Cse1 replaces r61 with r2 in just one insn:

--- foo.c.196r.cse1 ---
(insn 10 9 11 3 (set (reg:DI 2 %r2)
(reg:DI 2 %r2)) /home/vogt/foo.c:5 897 {*movdi_64}
 (expr_list:REG_DEAD (reg/v:DI 61 [ x ])
(nil)))
--- foo.c.196r.cse1 ---

Ciao

Dominik ^_^  ^_^

-- 

Dominik Vogt
IBM Germany


foo.tgz
Description: application/gtar-compressed
>From a7fc5b13c7f0eb59e0c0d1389d031acfae3f7c6b Mon Sep 17 00:00:00 2001
From: Dominik Vogt 
Date: Mon, 12 Oct 2015 13:05:46 +0100
Subject: [PATCH] !!!attempt fix

---
 gcc/cse.c | 13 +
 1 file changed, 13 insertions(+)

diff --git a/gcc/cse.c b/gcc/cse.c
index a9cc26a..74daf12 100644
--- a/gcc/cse.c
+++ b/gcc/cse.c
@@ -4600,10 +4600,16 @@ cse_insn (rtx_insn *insn)
   /* Set nonzero if we need to call force_const_mem on with the
 	 contents of src_folded before using it.  */
   int src_folded_force_flag = 0;
+  bool set_hard_reg_from_pseudo;
 
   dest = SET_DEST (sets[i].rtl);
   src = SET_SRC (sets[i].rtl);
 
+  set_hard_reg_from_pseudo = (src && REG_P (src)
+  && REGNO (src) >= FIRST_PSEUDO_REGISTER
+  && dest && REG_P (dest)
+  && REGNO (dest) < FIRST_PSEUDO_REGISTER);
+
   /* If SRC is a constant that has no machine mode,
 	 hash it with the destination's machine mode.
 	 This way we can keep different modes separate.  */
@@ -5168,6 +5174,13 @@ cse_insn (rtx_insn *insn)
 	  src_elt_cost = MAX_COST;
 	}
 
+	  if (set_hard_reg_from_pseudo
+	  && trial && REG_P (trial) && REGNO (trial) == REGNO (dest))
+	/* The register allocator is expected to do a better job of
+	   replacing pseudo registers with hard registers a la
+	   (set hardreg pseudoreg) -> (set hardreg hardreg)  */
+	continue;
+
 	  /* Avoid creation of overlapping memory moves.  */
 	  if (MEM_P (trial) && MEM_P (SET_DEST (sets[i].rtl)))
 	{
-- 
2.3.0



Re: GCC 5 Status Report (2015-01-19), Trunk in Stage 4

2015-01-27 Thread Dominik Vogt
On Tue, Jan 27, 2015 at 10:12:15AM +0100, Richard Biener wrote:
> On Tue, 27 Jan 2015, Jakub Jelinek wrote:
> > On Tue, Jan 27, 2015 at 10:04:38AM +0100, Richard Biener wrote:
> > > On Tue, 27 Jan 2015, Andreas Krebbel wrote:
> > > > [PATCH] S/390: -mhotpatch v2
> > > > https://gcc.gnu.org/ml/gcc-patches/2015-01/msg02370.html
> > > > 
> > > > It is a backend only change to our existing -mhotpatch feature
> > > > requested by the Linux kernel guys for the ftrace implementation:
> > > > https://lkml.org/lkml/2015/1/26/320
> > > > 
> > > > They need it in an upstream GCC asap. If we don't get it into 5.0 we
> > > > probably would need to commit it onto 5.1 branch right after the
> > > > release. I would rather try to avoid this since it would make the
> > > > hotpatch feature incompatible between 5.0 and 5.1.

> > > Did you consider using an alternate option name instead of changing
> > > it in an incompatible way?  I realize SUSE will need to backport this
> > 
> > Yeah, the option incompatibility worries me.  Can't -mhotpatch without =
> > stand for the old behavior?  Does it map to some -mhotpatch=X,Y value,
> > or is it not worth to support both?
> 
> It maps to -mhotpatch=12.

The new expression for the old behaviour would be -mhotpatch=12,2
(or attribute((hotpatch(12,2))), and -mno-notpatch is expressed as
-mhotpatch=0,0 now.

> The old one had one argument while the new
> one has two...  so eventually you can distinguish them this way,
> though for the inlining I'd have added -minline-hotpatched and
> if you switch the arguments of the new hotpatch then -mhotpatch=12
> would trivially become supported again...

The reasons for the incompatible changes are:

 - With the old code it was really weird that you requested n
   halfwords to be patched before the label and silently got two
   halfwords patched after the label too.

 - The interaction between inlining and hotpatching was really
   awkward to code and led to behaviour hard to understand for the
   user.  Furthermore hotpatching is now compatible with
   always_inline (which generated an error in the old code).

 - With the simplification to the interaction between inlining and
   hotpatching (needed by the kernel), the new code would only look
   the same on the command line but actually do something else.

 - The old code had already three options (-mhotpatch,
   -mno-hotpatch and -mhotpatch=) and adding another one to define
   the size to be patched after the label would have made an even
   bigger mess of the already bad usability.

 - The user who requested that feature does not use it yet and
   will get the updated patch with the new semantics before he
   starts to use it.

 - We believe that nobody has tried to use the old semantics
   (except the colleague next door who requested the new
   functionality for use in the kernel).

To sum it up, given that we don't expect any current users, making
an incompatible change and thus avoiding the extra hassle in
favour of a clean design seemed acceptable.  It would certainly be
possible to keep the interfaces compatible, but we think it is not
worth the effort.

Ciao

Dominik ^_^  ^_^

-- 

Dominik Vogt
IBM Germany



Question about synchronising libffi

2014-11-04 Thread Dominik Vogt
Whats the correct way to get patches from the libffi repository
into gcc?  As far as I understand, there are currently patches in
libffi that are not in gcc and vice versa.

I have a patch that adds complex type support to the reflection
library in libgo, but it requires patches from upstream libffi to
work.

Ciao

Dominik ^_^  ^_^

-- 

Dominik Vogt
IBM Germany



Re: Recent go changes broke alpha bootstrap

2014-11-03 Thread Dominik Vogt
On Thu, Oct 30, 2014 at 08:05:14AM -0700, Ian Taylor wrote:
> On Thu, Oct 30, 2014 at 5:46 AM, Dominik Vogt  wrote:
> > I'm not quite sure about the best approach.  The attempt to
> > represent C unions in the "right" way is ultimately futile as Go
> > does not have the types necessary for it.  For example, the
> > handling of anonymous bit fields will never be right as it's
> > undefinied.  On the other hand I could fix the issue at hand by
> > changing the way anonymous unions are represented in Go.
> >
> > Example:
> >
> >   struct { int8_t x; union { int16_t y; int 32_t z; }; };
> >
> > Was represented (before the patch) as
> >
> >   struct { X byte; int16 Y; }
> >
> > which had size 4, alignment 2 and y at offset 2 but should had
> > have size 8, alignment 4 and y at offset 4.  With the current
> > patch the Go layout is
> >
> >   struct { X byte; artificial_name struct { y [2]byte; align [0]int32; } }
> >
> > with the proper size, alignment and offset, but y is addressed as
> > ".artificial_name.y" insted of just ".y", and y is a byte array
> > and not an int16.
> >
> > I could remove the "artificial_name struct" and add padding before
> > and after y instead:
> >
> >   struct { X byte; pad_0 [3]byte; Y int16; pad_1 [2]byte; align [0]int32; }
> >
> > What do you think?
> 
> Sounds good to me.  Basically the fields of the anonymous union should
> be promoted to become fields of the struct.  We can't do it in
> general, but we can do it for the first field.  That addresses the
> actual uses of anonymous unions.

The attached patch fixes this, at least if the first element of the
union is not a bitfield.

Bitfields can really not be represented properly in Go (think about
constructs like "struct { int : 1; int bf : 1; }"), I'd rather not
try to represent them in a predictable way.  The patched code may
or may not give them a name, and reserves the proper amount of
padding where required in structs.  If a union begins with an
anonymous bitfield (which makes no sense), that is ignored.  If a
union begins with a named bitfield (possibly after unnamed ones),
this may or may not be used as the (sole) element of the Go
struct.

Ciao

Dominik ^_^  ^_^

-- 

Dominik Vogt
IBM Germany
>From 52d6792fb89a2f2beb2b897c7794b30245400e61 Mon Sep 17 00:00:00 2001
From: Dominik Vogt 
Date: Mon, 3 Nov 2014 17:49:33 +0100
Subject: [PATCH 1/2] godump: "Fix" handling of anonymous structs and unions.

Remove the "struct { ... }" around them as was the case before.

Note that bitfields are note handled in any predictable way.  They may be
replaced by padding or not, and a union beginning with a bitfield may use the
bitfield in the Go struct or may use the first non-bitfield element of the
union.  This is not really fixable.
---
 gcc/godump.c| 183 +++
 gcc/testsuite/gcc.misc-tests/godump-1.c | 849 +++-
 2 files changed, 697 insertions(+), 335 deletions(-)

diff --git a/gcc/godump.c b/gcc/godump.c
index fccd3eb..7c386c4 100644
--- a/gcc/godump.c
+++ b/gcc/godump.c
@@ -678,11 +678,13 @@ go_force_record_alignment (struct obstack *ob, const char *type_string,
 
 static bool
 go_format_type (struct godump_container *container, tree type,
-		bool use_type_name, bool is_func_ok, unsigned int *p_art_i)
+		bool use_type_name, bool is_func_ok, unsigned int *p_art_i,
+		bool is_anon_record_or_union)
 {
   bool ret;
   struct obstack *ob;
   unsigned int art_i_dummy;
+  bool is_union = false;
 
   if (p_art_i == NULL)
 {
@@ -856,7 +858,7 @@ go_format_type (struct godump_container *container, tree type,
   else
 	{
 	  if (!go_format_type (container, TREE_TYPE (type), use_type_name,
-			   true, NULL))
+			   true, NULL, false))
 	ret = false;
 	}
   break;
@@ -882,15 +884,19 @@ go_format_type (struct godump_container *container, tree type,
 	obstack_1grow (ob, '0');
   obstack_1grow (ob, ']');
   if (!go_format_type (container, TREE_TYPE (type), use_type_name, false,
-			   NULL))
+			   NULL, false))
 	ret = false;
   break;
 
+case UNION_TYPE:
+  is_union = true;
+  /* Fall through to RECORD_TYPE case.  */
 case RECORD_TYPE:
   {
 	unsigned int prev_field_end;
-	unsigned int most_strict_known_alignment;
+	unsigned int known_alignment;
 	tree field;
+	bool emitted_a_field;
 
 	/* FIXME: Why is this necessary?  Without it we can get a core
 	   dump on the s390x headers, or from a file containing simply
@@ -898,51 +904,77 @@ go_format_type (struct godump_container *container, tree type,
 	layout_type (type);
 
 	prev_field_end = 0;
-	most_strict_known_alignment = 1;
-	obstack_grow (ob, "struct { &q

Latest trunk does not build

2014-11-03 Thread Dominik Vogt
The latest trunk code does not build (on s390x):

  git commit id: 2ad7e37ad4be8621eade1f90dd2bc8124034712e
  git-svn-id: svn+ssh://gcc.gnu.org/svn/gcc/trunk@217039 
138bc75d-0d04-0410-961f-82ee72b054a4
  
Error messages:

-- snip --
g++ -c   -g3 -O3 -DIN_GCC-fno-exceptions -fno-rtti 
-fasynchronous-unwind-tables -W -Wall -Wno-narrowing -Wwrite-strings 
-Wcast-qual -Wmissing-format-attribute -Woverloaded-virtual -pedantic 
-Wno-long-long -Wno-variadic-macros -Wno-overlength-strings -fno-common  
-DHAVE_CONFIG_H -I. -I. -I../../gcc -I../../gcc/. -I../../gcc/../include 
-I../../gcc/../libcpp/include  -I../../gcc/../libdecnumber 
-I../../gcc/../libdecnumber/dpd -I../libdecnumber -I../../gcc/../libbacktrace   
 -o ifcvt.o -MT ifcvt.o -MMD -MP -MF ./.deps/ifcvt.TPo ../../gcc/ifcvt.c
In file included from ./tm.h:19:0,
 from ../../gcc/ifcvt.c:23:
./options.h:263:36: error: token "." is not valid in preprocessor expressions
 #define target_flags global_options.x_target_flags
^
./options.h:4276:29: note: in expansion of macro ‘target_flags’
 #define TARGET_HARD_FLOAT ((target_flags & MASK_SOFT_FLOAT) == 0)
 ^
./insn-flags.h:439:26: note: in expansion of macro ‘TARGET_HARD_FLOAT’
 #define HAVE_cbranchcc4 (TARGET_HARD_FLOAT)
  ^
../../gcc/ifcvt.c:1467:5: note: in expansion of macro ‘HAVE_cbranchcc4’
 #if HAVE_cbranchcc4
 ^
./options.h:263:36: error: token "." is not valid in preprocessor expressions
 #define target_flags global_options.x_target_flags
^
./options.h:4276:29: note: in expansion of macro ‘target_flags’
 #define TARGET_HARD_FLOAT ((target_flags & MASK_SOFT_FLOAT) == 0)
 ^
./insn-flags.h:439:26: note: in expansion of macro ‘TARGET_HARD_FLOAT’
 #define HAVE_cbranchcc4 (TARGET_HARD_FLOAT)
  ^
../../gcc/ifcvt.c:1799:5: note: in expansion of macro ‘HAVE_cbranchcc4’
 #if HAVE_cbranchcc4
 ^
./options.h:263:36: error: token "." is not valid in preprocessor expressions
 #define target_flags global_options.x_target_flags
^
./options.h:4276:29: note: in expansion of macro ‘target_flags’
 #define TARGET_HARD_FLOAT ((target_flags & MASK_SOFT_FLOAT) == 0)
 ^
./insn-flags.h:439:26: note: in expansion of macro ‘TARGET_HARD_FLOAT’
 #define HAVE_cbranchcc4 (TARGET_HARD_FLOAT)
  ^
../../gcc/ifcvt.c:2381:5: note: in expansion of macro ‘HAVE_cbranchcc4’
 #if HAVE_cbranchcc4
 ^
-- snip --

Ciao

Dominik ^_^  ^_^

-- 

Dominik Vogt
IBM Germany



Re: Recent go changes broke alpha bootstrap

2014-10-30 Thread Dominik Vogt
On Thu, Oct 30, 2014 at 11:11:09AM +0100, Uros Bizjak wrote:
> On Thu, Oct 30, 2014 at 8:40 AM, Uros Bizjak  wrote:
> > Recent go changes broke alpha bootstrap:
> 
> > $files/space/homedirs/uros/gcc-svn/trunk/libgo/go/os/stat_atim.go:22:29:
> > error: reference to undefined field or method ‘Mtim’
> >modTime: timespecToTime(st.Mtim),
> >  ^
> > /space/homedirs/uros/gcc-svn/trunk/libgo/go/os/stat_atim.go:60:50:
> > error: reference to undefined field or method ‘Atim’
> >   return timespecToTime(fi.Sys().(*syscall.Stat_t).Atim)

Hrm, this code relies on the old way of converting unions to Go
structures which, replaced the union with its first element
converted to a Go type.  I.e. the resulting size of the Go
representation of the union was wrong.

So, there was a kind hack that allowed to address the first field
of an anonymous union as if it was a plain field of the
surrounding structure (similar to C), and existing code now relies
on that.

I'm not quite sure about the best approach.  The attempt to
represent C unions in the "right" way is ultimately futile as Go
does not have the types necessary for it.  For example, the
handling of anonymous bit fields will never be right as it's
undefinied.  On the other hand I could fix the issue at hand by
changing the way anonymous unions are represented in Go.

Example:

  struct { int8_t x; union { int16_t y; int 32_t z; }; };

Was represented (before the patch) as

  struct { X byte; int16 Y; }

which had size 4, alignment 2 and y at offset 2 but should had
have size 8, alignment 4 and y at offset 4.  With the current
patch the Go layout is

  struct { X byte; artificial_name struct { y [2]byte; align [0]int32; } }

with the proper size, alignment and offset, but y is addressed as
".artificial_name.y" insted of just ".y", and y is a byte array
and not an int16.

I could remove the "artificial_name struct" and add padding before
and after y instead:

  struct { X byte; pad_0 [3]byte; Y int16; pad_1 [2]byte; align [0]int32; }

What do you think?

Ciao

Dominik ^_^  ^_^

-- 

Dominik Vogt
IBM Germany



Turning a single warning into an error in dejagnu test

2014-08-27 Thread Dominik Vogt
I'm writing a dejagnu test and encounter this warning at one place:

  warning: passing argument 1 of '...' makes integer from pointer
  without a cast [enabled by default]

Now, I have a "{ dg-error ... }" comment in that line.  The line
is generated from a script among hundreds of others that are all
expected to produce errors, not warnings.  It would be very
inconvenient (= lots of work) to change the script to make an
exception just for that single line (because there's no easy way
to identify lines that produce the warning instead of an error).

So the question is:  Is it possible to turn only this one warning
into an error inside a dejagnu test?  As I understand it, there
are no -W... switches for "enabled by default" options, and I
cannot use -Werror because that would break other tests in the
file.

Ciao

Dominik ^_^  ^_^

-- 

Dominik Vogt
IBM Germany



"Parallel" mode iterators

2014-08-21 Thread Dominik Vogt
Maybe some can help me a little bit with machine descriptions.
Assuming there is a define_insn pattern "foo", and that
pattern takes two arguments.  The first argument sould be of the
types DI, SI or HI, and the second argument is always half the
size of the first argument.

One can define mode iterators for

  (define_mode_iterator ITER1 [DI SI HI])
  (define_mode_iterator ITER2 [SI HI QI])

Is it possible to write something like this:

  (define_insn "foo" 
[(set (match_operand:ITER1 0 ...) 
 ...
[(match_operand:ITER1 1 ...)
 (match_operand:ITER2 2 ...)]
 ...

so that the pattern is copied only for the combinations DI-SI,
SI-HI and HI-QI, not for all nine combinations of the two
iterators?  (Or is there another way to get mode of the second
argument depending on the first argument?)

Ciao

Dominik ^_^  ^_^

-- 

Dominik Vogt
IBM Germany



Re: Need help: Is a VAR_DECL type builtin or not?

2014-02-17 Thread Dominik Vogt
On Fri, Feb 14, 2014 at 02:40:44PM +0100, Richard Biener wrote:
> On Fri, Feb 14, 2014 at 9:59 AM, Dominik Vogt  wrote:
> > Given a specific VAR_DECL tree node, I need to find out whether
> > its type is built in or not.  Up to now I have
> >
> >   tree tn = TYPE_NAME (TREE_TYPE (var_decl));
> >   if (tn != NULL_TREE && TREE_CODE (tn) == TYPE_DECL && DECL_NAME (tn))
> > {
> >   ...
> > }
> >
> > This if-condition is true for both,
> >
> >   int x;
> >   const int x;
> >   ...
> >
> > and
> >
> >   typedef int i_t;
> >   i_t x;
> >   const i_t x;
> >   ...
> >
> > I need to weed out the class of VAR_DECLs that directly use built
> > in types.
> 
> Try DECL_IS_BUILTIN.  But I question how you define "builtin" here?

Well, actually I'm working on the variable output function in
godump.c.  At the moment, if the code comes across

  typedef char c_t
  chat c1;
  c_t c2;

it emits

  type _c_t byte
  var c1 byte
  var c2 byte

This is fine for c1, but for c2 it should really use the type:

  var c2 _c_t

So the rule I'm trying to implement is:

  Given a Tree node that is a VAR_DECL, if its type is an "alias"
  (defined with typedef/union/struct/class etc.), use the name of
  the alias, otherwise resolve the type recursively until only
  types built into the language are left.

It's really only about the underlying data types (int, float,
_Complex etc.), not about storage classes, pointers, attributes,
qualifiers etc.

Well, since godump.c already caches all declarations it has come
across, I could assume that these declarations are not built-in
and use that in the "rule" above.

Ciao

Dominik ^_^  ^_^

-- 

Dominik Vogt
IBM Germany



Need help: Is a VAR_DECL type builtin or not?

2014-02-14 Thread Dominik Vogt
Given a specific VAR_DECL tree node, I need to find out whether
its type is built in or not.  Up to now I have

  tree tn = TYPE_NAME (TREE_TYPE (var_decl));
  if (tn != NULL_TREE && TREE_CODE (tn) == TYPE_DECL && DECL_NAME (tn))
{
  ...
}

This if-condition is true for both,

  int x;
  const int x;
  ...

and

  typedef int i_t;
  i_t x;
  const i_t x;
  ...

I need to weed out the class of VAR_DECLs that directly use built
in types.

Ciao

Dominik ^_^  ^_^

-- 

Dominik Vogt
IBM Germany