PATCH: Handle ZERO_EXTEND offsettable address

[H.J. Lu]

Hi,

In

(insn 19 17 20 2 (set (reg:TI 85 [ *_15 ])
(mem:TI (zero_extend:DI (reg:SI 82)) [0 *_15+0 S16 A32])) x.i:29 61
{*movti_internal_rex64}
 (expr_list:REG_DEAD (reg:SI 82) 
(expr_list:REG_EQUIV (mem/c:TI (plus:DI (reg/f:DI 20 frame)
(const_int -16 [0xfff0])) [0 sym+0 S16 A64])

we fail to see (mem:TI (zero_extend:DI (reg:SI 82))) is offsettable.
This patch adds ZERO_EXTEND support to adjust_address_1 and
offsettable_address_addr_space_p.  Tested on Linux/x32.  OK to install?

Thanks.


H.J.
---
gcc/

2012-11-10  H.J. Lu  

PR rtl-optimization/55247
PR middle-end/55259
* emit-rtl.c (adjust_address_1): Handle ZERO_EXTEND.
* recog.c (offsettable_address_addr_space_p): Likewise.

gcc/testsuite/

2012-11-10  H.J. Lu  

PR rtl-optimization/55247
PR middle-end/55259

diff --git a/gcc/emit-rtl.c b/gcc/emit-rtl.c
index 95bbfa7..d7c454c 100644
--- a/gcc/emit-rtl.c
+++ b/gcc/emit-rtl.c
@@ -2109,6 +2109,12 @@ adjust_address_1 (rtx memref, enum machine_mode mode, 
HOST_WIDE_INT offset,
addr = gen_rtx_LO_SUM (address_mode, XEXP (addr, 0),
   plus_constant (address_mode,
  XEXP (addr, 1), offset));
+  else if (GET_CODE (addr) == ZERO_EXTEND)
+   {
+ addr = XEXP (addr, 0);
+ addr = plus_constant (GET_MODE (addr), addr, offset);
+ addr = gen_rtx_ZERO_EXTEND (address_mode, addr);
+   }
   else
addr = plus_constant (address_mode, addr, offset);
 }
diff --git a/gcc/recog.c b/gcc/recog.c
index ee68e30..d3dd591 100644
--- a/gcc/recog.c
+++ b/gcc/recog.c
@@ -1934,15 +1934,21 @@ int
 offsettable_address_addr_space_p (int strictp, enum machine_mode mode, rtx y,
  addr_space_t as)
 {
-  enum rtx_code ycode = GET_CODE (y);
+  enum rtx_code ycode;
   rtx z;
-  rtx y1 = y;
+  rtx y1;
   rtx *y2;
   int (*addressp) (enum machine_mode, rtx, addr_space_t) =
 (strictp ? strict_memory_address_addr_space_p
 : memory_address_addr_space_p);
   unsigned int mode_sz = GET_MODE_SIZE (mode);
 
+  if (GET_CODE (y) == ZERO_EXTEND)
+y = XEXP (y, 0);
+
+  ycode = GET_CODE (y);
+  y1 = y;
+
   if (CONSTANT_ADDRESS_P (y))
 return 1;
 
diff --git a/gcc/testsuite/gcc.target/i386/pr55247.c 
b/gcc/testsuite/gcc.target/i386/pr55247.c
new file mode 100644
index 000..594139e
--- /dev/null
+++ b/gcc/testsuite/gcc.target/i386/pr55247.c
@@ -0,0 +1,35 @@
+/* { dg-do compile { target { ! { ia32 } } } } */
+/* { dg-require-effective-target maybe_x32 } */
+/* { dg-options "-O -mno-sse -mno-mmx -mx32 -maddress-mode=long" } */
+
+typedef unsigned int uint32_t;
+typedef uint32_t Elf32_Word;
+typedef uint32_t Elf32_Addr;
+typedef struct {
+  Elf32_Word st_name;
+  Elf32_Addr st_value;
+  Elf32_Word st_size;
+  unsigned char st_other;
+} Elf32_Sym;
+typedef struct {
+  Elf32_Word r_info;
+}
+Elf32_Rela;
+typedef struct {
+  union {
+Elf32_Addr d_ptr;
+  }
+  d_un;
+} Elf32_Dyn;
+struct link_map   {
+  Elf32_Dyn *l_info[34];
+};
+extern void symbind32 (Elf32_Sym *);
+void
+_dl_profile_fixup (struct link_map *l, Elf32_Word reloc_arg)
+{
+  const Elf32_Sym *const symtab  = (const void *) l->l_info[6]->d_un.d_ptr;
+  const Elf32_Rela *const reloc  = (const void *) (l->l_info[23]->d_un.d_ptr + 
reloc_arg * sizeof (Elf32_Rela));
+  Elf32_Sym sym = symtab[(reloc->r_info) >> 8];
+  symbind32 (&sym);
+}

[PATCH] Fix combined tree for LTO

[Andrew Pinski]

Hi,
  The problem here is that set PLUGIN_LD_SUFFIX to ld-new which is not
the final installed binary name.  This patch fixes the problem by
changing if we got ld-new to just ld.
Note this issue has been around since 4.6 but not many people test
installed binary and it does not show up while doing testing from the
combined tree either.

OK?  Bootstrapped and tested with and without being in a combined tree
without any regressions.

Thanks,
Andrew Pinski

* configure.ac: Set PLUGIN_LD_SUFFIX to just "ld" if it was "ld-new".
* configure: Regenerate.
diff --git a/gcc/configure b/gcc/configure
index 55164b6..bce98ad 100755
--- a/gcc/configure
+++ b/gcc/configure
@@ -21343,6 +21343,11 @@ fi
 
 ORIGINAL_PLUGIN_LD_FOR_TARGET=$gcc_cv_ld
 PLUGIN_LD_SUFFIX=`basename $gcc_cv_ld | sed -e "s,$target_alias-,,"`
+# if the PLUGIN_LD is set ld-new, just have it as ld
+# as that is the installed named.
+if test x$PLUGIN_LD_SUFFIX == xld-new; then
+  PLUGIN_LD_SUFFIX=ld
+fi
 
 # Check whether --with-plugin-ld was given.
 if test "${with_plugin_ld+set}" = set; then :
diff --git a/gcc/configure.ac b/gcc/configure.ac
index fbda2cc..39d8b43 100644
--- a/gcc/configure.ac
+++ b/gcc/configure.ac
@@ -2003,6 +2003,11 @@ fi])
 
 ORIGINAL_PLUGIN_LD_FOR_TARGET=$gcc_cv_ld
 PLUGIN_LD_SUFFIX=`basename $gcc_cv_ld | sed -e "s,$target_alias-,,"`
+# if the PLUGIN_LD is set ld-new, just have it as ld
+# as that is the installed named.
+if test x$PLUGIN_LD_SUFFIX == xld-new; then
+  PLUGIN_LD_SUFFIX=ld
+fi
 AC_ARG_WITH(plugin-ld,
 [AS_HELP_STRING([[--with-plugin-ld=[ARG]]], [specify the plugin linker])],
 [if test x"$withval" != x; then

[Committed] Add testcase

[Andrew Pinski]

Hi,
  While I was working on using zero_extract more on the left hand side
in combine, I found a bug in my code but there was no testcase in the
testsuite that tested it.  The code originally comes from uboot which
was failing when doing a ping or tftp.

Committed the testcase as obvious after a quick test to make sure it
works.  Note someone might need to mark the testcase as only
executable on targets which have 32bit ints.

Thanks,
Andrew Pinski

ChangeLog:
 * gcc.c-torture/execute/20121108-1.c: New testcase.
Index: gcc.c-torture/execute/20121108-1.c
===
--- gcc.c-torture/execute/20121108-1.c  (revision 0)
+++ gcc.c-torture/execute/20121108-1.c  (revision 0)
@@ -0,0 +1,51 @@
+char temp[] = "192.168.190.160";
+unsigned result = (192u<<8)|168u)<<8)|190u)<<8)|160u;
+
+int strtoul1(const char *a, char **b, int c) __attribute__((noinline, 
noclone));
+int strtoul1(const char *a, char **b, int c)
+{
+  *b = a+3;
+  if (a == temp)
+return 192;
+  else if (a == temp+4)
+return 168;
+  else if (a == temp+8)
+return 190;
+  else if (a == temp+12)
+return 160;
+  __builtin_abort();
+}
+
+int string_to_ip(const char *s) __attribute__((noinline,noclone));
+int string_to_ip(const char *s)
+{
+int addr;
+char *e;
+int i;
+
+if (s == 0)
+return(0);
+
+for (addr=0, i=0; i<4; ++i) {
+int val = s ? strtoul1(s, &e, 10) : 0;
+addr <<= 8;
+addr |= (val & 0xFF);
+if (s) {
+s = (*e) ? e+1 : e;
+}
+}
+
+return addr;
+}
+
+int main(void)
+{
+  int t = string_to_ip (temp);
+  printf ("%x\n", t);
+  printf ("%x\n", result);
+  if (t != result)
+__builtin_abort ();
+  printf ("WORKS.\n");
+  return 0;
+}
+

Re: [PATCH/MIPS] Use ins/dins instruction when written manually

[Andrew Pinski]

On Mon, 2012-11-05 at 11:20 -0800, Andrew Pinski wrote:
> On Mon, 2012-11-05 at 19:19 +, Richard Sandiford wrote:
> > Hi Andrew,
> > 
> > Andrew Pinski  writes:
> > > On Fri, Oct 5, 2012 at 8:43 PM, Andrew Pinski
> > >  wrote:
> > >> On Sun, Aug 19, 2012 at 10:13 AM, Richard Sandiford
> > >>  wrote:
> > >>> Andrew Pinski  writes:
> >    Right now we only produce ins when a zero_extract is used on the
> >  right hand side.  We can do better by adding some patterns which
> >  combine for the ins instruction.  This patch adds those patterns and a
> >  testcase which shows a simple example where the code is improved.
> > >>>
> > >>> Sorry for the delay in reviewing this.  Had you thought about trying to
> > >>> teach combine.c about this instead?  It doesn't look like any of the
> > >>> patterns are really providing more information about the underlying
> > >>> instruction.
> > >>
> > >> combine.c has some code to do this already if one of the src register
> > >> is the same as the dest register; that is what make_field_assignment
> > >> does.  Quickly looking at the code, the problem I doing it in
> > >> make_field_assignment is there is no way to return that you need a
> > >> copy of the value first unless I am missing something obvious.  Now I
> > >> agree we should be optimize this in combine rather than these manual
> > >> patterns.
> > >
> > > I now have a patch which implements this in combine which allows the
> > > backend not need to change.  I generate a SEQUENCE which then
> > > try_combine splits like we do for PARALLEL but keeping it in the
> > > correct order and allowing for the case where we are combing two
> > > instructions into two instructions.
> > > I hope to be able to post it later on Saturday.
> > 
> > Just wondering, what's the status of this?  Was worried that you might
> > have posted it and I'd missed it.
> 
> I have not posted it yet.  I am still cleaning up the code and making
> sure it does not cause regressions.

Hi Richard,
  Yesterday, I found out my approach is not ready for the rest of
combine.  So I have to go back to the drawing board of writing a patch.
I will commit the testcase which shows the regression as the testsuite
did not have a testcase for the issue before.

Thanks,
Andrew Pinski


> 
> Thanks,
> Andrew
> 
> > 
> > Richard
> > 
> 
> 
> 

Re: Asan/Tsan Unit/Regression testing (was [asan] Emit GIMPLE direclty, small cleanups)

[Konstantin Serebryany]

[re-sending so that it gets to the list]

asan's design choice is to hard crash on the first error.

Doing many asan checks in the same process might become too messy too soon.
I wouldn't try to implement it myself.
Playing with longjmp in asan is not the best idea because asan
intercepts longjmp
and in fact it does have some locks around __asan_report_*
That's why we really like the death tests that run every check in a
separate process.

Why depending on gtest is bad?

--kcc

On Fri, Nov 9, 2012 at 11:16 AM, Jakub Jelinek  wrote:
>
> On Fri, Nov 09, 2012 at 08:13:12PM +0100, Jakub Jelinek wrote:
> > On Fri, Nov 09, 2012 at 11:05:37AM -0800, Wei Mi wrote:
> > > gtest integrate multiple tests into the same file with each test being
> > > a single line check.  I cannot think out a method to migrate it to
> > > dejagnu without using gtest, except splitting a single gtest file to
> > > multiple files with each file per test. asan has about 130 tests so
> > > have to write 130 files which will be a doable but painful task.
> >
> > See the glibc _FORTIFY_SOURCE check I've referenced, there it is 3 lines
> > per test expected to crash, but could be done in a single macro too.
> > If the failure can be intercepted, it can be done in a single process
> > (e.g.
> > SIGABRT can, _exit can't that easily), otherwise perhaps say way to skip
> > previous tests and communicate with dejagnu how many times it should run
> > the
> > executable.
>
> And quick grep shows that one can call __sanitizer::SetDieCallback
> and supposedly, if ASAN doesn't hold any locks when it calls Die() that
> would prevent it from functioning further, we can just longjmp out from
> the
> callback.
>
> Jakub

Re: User directed Function Multiversioning via Function Overloading (issue5752064)

[Sriraman Tallam]

Hi Jason,

   Made all the changes and attached patch. Ok to commit?

Thanks,
-Sri.

On Tue, Nov 6, 2012 at 7:52 AM, Jason Merrill  wrote:
> On 11/05/2012 09:38 PM, Sriraman Tallam wrote:
>>
>> +  /* For multi-versioned functions, more than one match is just fine.
>>
>> +Call decls_match to make sure they are different because they are
>> +versioned.  */
>> +  if (DECL_FUNCTION_VERSIONED (fn))
>> +   {
>> +  for (match = TREE_CHAIN (matches); match; match = TREE_CHAIN
>> (match))
>> +   if (!DECL_FUNCTION_VERSIONED (TREE_PURPOSE (match))
>> +   || decls_match (fn, TREE_PURPOSE (match)))
>> + break;
>> +   }
>
>
> I still don't understand what this code is supposed to be doing.  Please
> remove it and instead modify the other loop to allow mismatches that are
> versions of the same function.
>
>> +  /* If the olddecl is a version, so is the newdecl.  */
>> +  if (TREE_CODE (newdecl) == FUNCTION_DECL
>> +  && DECL_FUNCTION_VERSIONED (olddecl))
>> +{
>> +  DECL_FUNCTION_VERSIONED (newdecl) = 1;
>> +  /* newdecl will be purged and is no longer a version.  */
>> +  delete_function_version (newdecl);
>> +}
>
>
> Please make the comment clearer that the reason we're setting the flag on
> the newdecl is so that it'll be copied back into the olddecl; otherwise it
> seems odd to say it's a version and then it isn't a version.
>
>> +  /* If a pointer to a function that is multi-versioned is requested, the
>> + pointer to the dispatcher function is returned instead.  This works
>> + well because indirectly calling the function will dispatch the right
>> + function version at run-time.  */
>>
>> +  if (DECL_FUNCTION_VERSIONED (fn))
>> +{
>> +  tree dispatcher_decl = NULL;
>> +  gcc_assert (targetm.get_function_versions_dispatcher);
>> +  dispatcher_decl = targetm.get_function_versions_dispatcher (fn);
>> +  if (!dispatcher_decl)
>> +   {
>> + error_at (input_location, "Pointer to a multiversioned function"
>> +   " without a default is not allowed");
>> + return error_mark_node;
>> +   }
>> +  retrofit_lang_decl (dispatcher_decl);
>> +  fn = dispatcher_decl;
>
>
> This code should use the get_function_version_dispatcher function in
> cp/call.c.
>
> Jason
>

* cgraph.c (insert_new_cgraph_node_version): Use cgraph_get_node
instead of cgraph_get_create_node.
* cp/class.c (mark_versions_used): Remove.
(resolve_address_of_overloaded_function): Do not call decls_match
for versioned functions. Call get_function_versions_dispatcher.
* cp/decl.c (duplicate_decls): Add comments.
* cp/call.c (get_function_version_dispatcher): Expose function.
(mark_versions_used): Expose function.
* cp/cp-tree.h (mark_versions_used): New declaration.
(get_function_version_dispatcher): Ditto.
* testsuite/g++.dg/mv4.C: Add require ifunc. Change error message.
* testsuite/g++.dg/mv5.C: Add require ifunc.
* testsuite/g++.dg/mv6.C: Add require ifunc.

Index: cgraph.c
===
--- cgraph.c(revision 193385)
+++ cgraph.c(working copy)
@@ -206,7 +206,7 @@ insert_new_cgraph_node_version (struct cgraph_node
 void
 delete_function_version (tree decl)
 {
-  struct cgraph_node *decl_node = cgraph_get_create_node (decl);
+  struct cgraph_node *decl_node = cgraph_get_node (decl);
   struct cgraph_function_version_info *decl_v = NULL;
 
   if (decl_node == NULL)
Index: testsuite/g++.dg/mv4.C
===
--- testsuite/g++.dg/mv4.C  (revision 193385)
+++ testsuite/g++.dg/mv4.C  (working copy)
@@ -3,6 +3,7 @@
and its pointer is taken.  */
 
 /* { dg-do compile { target i?86-*-* x86_64-*-* } } */
+/* { dg-require-ifunc "" }  */
 /* { dg-options "-O2 -mno-sse -mno-popcnt" } */
 
 int __attribute__ ((target ("sse")))
@@ -18,6 +19,6 @@ foo ()
 
 int main ()
 {
-  int (*p)() = &foo; /* { dg-error "Pointer to a multiversioned function 
without a default is not allowed" {} } */
+  int (*p)() = &foo; /* { dg-error "Pointer/Call to multiversioned function 
without a default is not allowed" {} } */
   return (*p)();
 }
Index: testsuite/g++.dg/mv6.C
===
--- testsuite/g++.dg/mv6.C  (revision 193385)
+++ testsuite/g++.dg/mv6.C  (working copy)
@@ -1,6 +1,7 @@
 /* Test to check if member version multiversioning works correctly.  */
 
 /* { dg-do run { target i?86-*-* x86_64-*-* } } */
+/* { dg-require-ifunc "" }  */
 
 class Foo
 {
Index: testsuite/g++.dg/mv5.C
===
--- testsuite/g++.dg/mv5.C  (revision 193385)
+++ testsuite/g++.dg/mv5.C  (working copy)
@@ -2,6 +2,7 @@
marked comdat with inline keyword.  */
 
 /* { dg-do run { ta

Re: libstdc++ PATCH to use __cplusplus rather than __GXX_EXPERIMENTAL_CXX0X__

[Ed Smith-Rowland]

On 11/09/2012 05:09 PM, Jason Merrill wrote:
Now that G++ uses the value of __cplusplus specified by the standard, 
we don't need the other macro anymore.


OK for trunk, or should I save it for the next stage 1?

Jason
This looks like a non-user-facing mechanical change that makes sense.  
FWIW I'm OK with it.


Paolo?

Ed

Re: [asan] Patch - fix an ICE in asan.c

[Tobias Burnus]

Tobias Burnus wrote:

The attached test case ICEs (segfault) both on the asan branch and on
the trunk with Dodji's patches:


I found another ICE - this time without a patch.

[That's with the patch, which I posted in this thread. Without, one 
seems to run into the problem I tried to fix with the patch.]


[As ASAN is not yet in the trunk, it is not yet suitable for a PR - but 
on the other hand, I am afraid that I loose it. Thus, I dump it here, 
which is also not the best place (sorry).]



The attached code generates (before ASAN):

StringSwitch& StringSwitch ...
...
  :
  _2 = &this_1(D)->Str;
  _3 = StringRef::data (_2);
  memcmp (S_4(D), _3, 7);
  return;
}

And within this basic block, between "_3" and "memcpy", the generated 
ASAN code is added, which leads to an ICE and 10 times the message.



error: control flow in the middle of basic block 7


If one looks at the asan0 dump (after disabling this part of checking), 
one finds:


  _52 = _48 & _51;
  if (_52 != 0)
  _53 = (unsigned long) _22;

The "if" line looks odd as one would expect code of this form:

  if (_62 != 0)
goto ;
  else
goto ;

See attachments.

Tobias
extern "C"
{
  extern int memcmp (const void *__s1, const void *__s2,
 long unsigned int __n) throw ();
}
struct StringRef
{
  const char *data () const { }
};
template < typename T, typename R = T > class StringSwitch
{
  StringRef Str;
  public:
 explicit StringSwitch (StringRef Str):Str (Str) { }
  template < unsigned N > StringSwitch & Case (const char (&S)[N])
  {
memcmp (S, Str.data (), N - 1);
  }
  R Default () const { }
};

int getIt (StringRef Name)
{
  return StringSwitch < int > (Name)
.Case ("unknown")
.Default ();
}

;; Function const char* StringRef::data() const (_ZNK9StringRef4dataEv, 
funcdef_no=0, decl_uid=2200, cgraph_uid=0)

const char* StringRef::data() const (const struct StringRef * const this)
{
  :
  GIMPLE_NOP
  return;

}



;; Function int getIt(StringRef) (_Z5getIt9StringRef, funcdef_no=4, 
decl_uid=2230, cgraph_uid=1)

int getIt(StringRef) (struct StringRef Name)
{
  struct StringSwitch & D.2293;
  struct StringRef D.2292;
  struct StringRef D.2277;
  struct StringSwitch D.2278;
  int D.2291;
  struct StringSwitch & _1;
  int _2;

  :
  StringSwitch::StringSwitch (&D.2278, D.2292);
  _1 = StringSwitch::Case<8u> (&D.2278, "unknown");
  _2 = StringSwitch::Default (_1);
  D.2278 ={v} {CLOBBER};

:
  return _2;

}



;; Function StringSwitch::StringSwitch(StringRef) [with T = int; R = int] 
(_ZN12StringSwitchIiiEC2E9StringRef, funcdef_no=6, decl_uid=2249, cgraph_uid=3)

StringSwitch::StringSwitch(StringRef) [with T = int; R = int] (struct 
StringSwitch * const this, struct StringRef Str)
{
  :
  return;

}



;; Function StringSwitch& StringSwitch::Case(const char (&)[N]) 
[with unsigned int N = 8u; T = int; R = int] 
(_ZN12StringSwitchIiiE4CaseILj8EEERS0_RAT__Kc, funcdef_no=8, decl_uid=2279, 
cgraph_uid=5)

StringSwitch& StringSwitch::Case(const char (&)[N]) [with unsigned 
int N = 8u; T = int; R = int] (struct StringSwitch * const this, const char[8] 
& S)
{
  const char * D.2297;
  struct StringRef * D.2296;
  struct StringRef * _2;
  const char * _3;
  unsigned long _5;
  unsigned long _6;
  unsigned long _7;
  signed char * _8;
  signed char _9;
  bool _10;
  unsigned long _11;
  signed char _12;
  bool _13;
  bool _14;
  long unsigned int _15;
  long unsigned int _16;
  const char[8] & _17;
  const char[8] & _18;
  unsigned long _19;
  unsigned long _20;
  unsigned long _21;
  signed char * _22;
  signed char _23;
  bool _24;
  unsigned long _25;
  signed char _26;
  bool _27;
  bool _28;
  unsigned long _29;
  unsigned long _30;
  unsigned long _31;
  signed char * _32;
  signed char _33;
  bool _34;
  unsigned long _35;
  signed char _36;
  bool _37;
  bool _38;
  long unsigned int _39;
  long unsigned int _40;
  const char * _41;
  const char * _42;
  unsigned long _43;
  unsigned long _44;
  unsigned long _45;
  signed char * _46;
  signed char _47;
  bool _48;
  unsigned long _49;
  signed char _50;
  bool _51;
  bool _52;
  unsigned long _53;
  unsigned long _54;
  unsigned long _55;
  signed char * _56;
  signed char _57;
  bool _58;
  unsigned long _59;
  signed char _60;
  bool _61;
  bool _62;

  :
  _2 = &this_1(D)->Str;
  _3 = StringRef::data (_2);
  _5 = (unsigned long) S_4(D);
  _6 = _5 >> 3;
  _7 = _6 + 17592186044416;
  _8 = (signed char *) _7;
  _9 = *_8;
  _10 = _9 != 0;
  _11 = _5 & 7;
  _12 = (signed char) _11;
  _13 = _12 >= _9;
  _14 = _10 & _13;
  if (_14 != 0)
goto ;
  else
goto ;

  :
  __asan_report_load1 (_5);

  :
  _29 = (unsigned long) _3;
  _30 = _29 >> 3;
  _31 = _30 + 17592186044416;
  _32 = (signed char *) _31;
  _33 = *_32;
  _34 = _33 != 0;
  _35 = _29 & 7;
  _36 = (signed char) _35;
  _37 = _36 >= _33;
  _38 = _34 & _37;
  if (_38 != 0)
goto ;
  else
goto ;

  :
  __asan_report_load1 (_29);

  :
  _39 = 7;
  _40 = _39 - 1;
  _41 = _3;
  _42 = _41 + _40;

[Ada] Fix PR other/52438

[Eric Botcazou]

This removes s-taspri-lynxos.ads, which is unused, and converts the 3 last 
files under GPL V2+ (plus exception) to GPL V3+ (plus exception), as all the 
other similar files.  Applied on mainline and 4.7 branch.


2012-11-09  Eric Botcazou  

PR other/52438
* s-taspri-lynxos.ads: Delete.
* s-osinte-kfreebsd-gnu.ads: Change license to GPL V3+.
* s-tpopsp-rtems.adb: Likewise.
* s-osinte-rtems.adb: Likewise.


-- 
Eric BotcazouIndex: s-osinte-kfreebsd-gnu.ads
===
--- s-osinte-kfreebsd-gnu.ads	(revision 193322)
+++ s-osinte-kfreebsd-gnu.ads	(working copy)
@@ -9,23 +9,21 @@
 -- Copyright (C) 1991-1994, Florida State University--
 -- Copyright (C) 1995-2005,2008 Free Software Foundation, Inc.  --
 --  --
--- GNARL is free software; you can  redistribute it  and/or modify it under --
+-- GNAT is free software;  you can  redistribute it  and/or modify it under --
 -- terms of the  GNU General Public License as published  by the Free Soft- --
--- ware  Foundation;  either version 2,  or (at your option) any later ver- --
--- sion. GNARL is distributed in the hope that it will be useful, but WITH- --
+-- ware  Foundation;  either version 3,  or (at your option) any later ver- --
+-- sion.  GNAT is distributed in the hope that it will be useful, but WITH- --
 -- OUT ANY WARRANTY;  without even the  implied warranty of MERCHANTABILITY --
--- or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License --
--- for  more details.  You should have  received  a copy of the GNU General --
--- Public License  distributed with GNARL; see file COPYING.  If not, write --
--- to  the  Free Software Foundation,  51  Franklin  Street,  Fifth  Floor, --
--- Boston, MA 02110-1301, USA.  --
+-- or FITNESS FOR A PARTICULAR PURPOSE. --
 --  --
--- As a special exception,  if other files  instantiate  generics from this --
--- unit, or you link  this unit with other files  to produce an executable, --
--- this  unit  does not  by itself cause  the resulting  executable  to  be --
--- covered  by the  GNU  General  Public  License.  This exception does not --
--- however invalidate  any other reasons why  the executable file  might be --
--- covered by the  GNU Public License.  --
+-- As a special exception under Section 7 of GPL version 3, you are granted --
+-- additional permissions described in the GCC Runtime Library Exception,   --
+-- version 3.1, as published by the Free Software Foundation.   --
+--  --
+-- You should have received a copy of the GNU General Public License and--
+-- a copy of the GCC Runtime Library Exception along with this program; --
+-- see the files COPYING3 and COPYING.RUNTIME respectively.  If not, see--
+-- .  --
 --  --
 -- GNARL was developed by the GNARL team at Florida State University.   --
 -- Extensive contributions were provided by Ada Core Technologies, Inc. --
Index: s-taspri-lynxos.ads
===
--- s-taspri-lynxos.ads	(revision 193322)
+++ s-taspri-lynxos.ads	(working copy)
@@ -1,125 +0,0 @@
---
---  --
---  GNAT RUN-TIME LIBRARY (GNARL) COMPONENTS--
---  --
--- S Y S T E M . T A S K _ P R I M I T I V E S  --
---  --
---  S p e c --
---  --
--- Copyright (C) 1991-1994, Florida State University--
--- Copyright (C) 1995-2008, AdaCore --
---  --
--- GNARL is free software; you can  redistribute it  and/or modify it under --
--- terms of the  GNU General Public License as published  by the Free Soft- --
--- ware  Foundation;  either version 2,  or (at your option) any later ver- --
--- sion. GNARL is distributed in the hope that it will be useful, but WITH- --
--- OUT ANY WARRANTY;  without even the  implied warranty of MERCHANTABILITY --
--- or FITNESS FOR A PARTICULAR PURPOSE.  See

lra branch merged with trunk

[Vladimir Makarov]

I've merged latest changes made on the trunk (up to rev. 193365) into 
lra branch.

C++ PATCH for c++/54859 (bogus errors with alias templates)

[Jason Merrill]

My patch for 54575 started using instantiate_template for alias 
templates, but instantiate_template wasn't prepared to instantiate with 
dependent template arguments.  Fixed thus.


Tested x86_64-pc-linux-gnu, applied to trunk.
commit 11ab97f0fe9a7262362f4ef997562f32028e18e4
Author: Jason Merrill 
Date:   Thu Nov 8 16:17:41 2012 -0500

	PR c++/54859
	* pt.c (check_instantiated_arg): Don't complain about dependent args.

diff --git a/gcc/cp/pt.c b/gcc/cp/pt.c
index 1ff1c73..50d12b0 100644
--- a/gcc/cp/pt.c
+++ b/gcc/cp/pt.c
@@ -14362,6 +14362,8 @@ tsubst_copy_and_build (tree t,
 static bool
 check_instantiated_arg (tree tmpl, tree t, tsubst_flags_t complain)
 {
+  if (dependent_template_arg_p (t))
+return false;
   if (ARGUMENT_PACK_P (t))
 {
   tree vec = ARGUMENT_PACK_ARGS (t);
diff --git a/gcc/testsuite/g++.dg/cpp0x/alias-decl-25.C b/gcc/testsuite/g++.dg/cpp0x/alias-decl-25.C
new file mode 100644
index 000..e72bd35
--- /dev/null
+++ b/gcc/testsuite/g++.dg/cpp0x/alias-decl-25.C
@@ -0,0 +1,10 @@
+// PR c++/54859
+// { dg-options -std=c++11 }
+
+template
+  using Num = int;
+
+template
+  using Count = Num;
+
+Count i;

Re: [PATCH] Handle abortTransaction with RTM

[Andi Kleen]

> I'm not sure this is quite true.  If a libitm-executed transaction is

It's just a convention. You don't have to use it. Not doing it will
just make abort profiling harder.

-Andi

[asan] Patch - fix an ICE in asan.c

[Tobias Burnus]

The attached test case ICEs (segfault) both on the asan branch and on 
the trunk with Dodji's patches:


fail31.ii: In static member function 'static std::size_t 
std::char_traits::length(const char_type*)':

fail31.ii:13:19: internal compiler error: Segmentation fault
 static size_t length (const char_type * __s)
   ^
0xae02ef crash_signal
/projects/tob/gcc-git/gcc/gcc/toplev.c:334
0xaf031d gsi_next
/projects/tob/gcc-git/gcc/gcc/gimple.h:5072
0xaf031d transform_statements
/projects/tob/gcc-git/gcc/gcc/asan.c:1357
0xaf031d asan_instrument
/projects/tob/gcc-git/gcc/gcc/asan.c:1556



The problem is in asa.c's transform_statements:

  FOR_EACH_BB (bb)
{
  if (bb->index >= saved_last_basic_block) continue;
  for (i = gsi_start_bb (bb); !gsi_end_p (i); gsi_next (&i))
{
  gimple s = gsi_stmt (i);

  if (gimple_assign_single_p (s))
instrument_assignment (&i);
  else if (is_gimple_call (s))
maybe_instrument_call (&i);
}


Here, "gsi_end_p(i)" is the check "i->ptr == NULL" and gsi_next(&i) is 
"i->ptr = i->ptr->gsbase.next;"


Thus, it looks fine at a glance. However, the problem is that the 
gsi_end_p check is done before the loop body while "gsi_next" is called 
after the loop body. That's fine unless "i" is modified in between, 
which happens in


instrument_strlen_call (gimple_stmt_iterator *iter)
...
  gimple_stmt_iterator gsi = *iter;
...
  *iter = gsi;
}

After the call, iter->ptr == NULL.


Is the patch okay for the ASAN branch?*

Tobias

* I still have to do an all-language bootstrap and regtesting, though 
the latter is probably pointless as there is currently not a single 
-fasan test case.
--- gcc/asan.c.orig	2012-11-09 21:26:26.0 +0100
+++ gcc/asan.c	2012-11-09 21:26:00.0 +0100
@@ -1362,6 +1362,8 @@ transform_statements (void)
 	instrument_assignment (&i);
 	  else if (is_gimple_call (s))
 	maybe_instrument_call (&i);
+	  if (gsi_end_p (i))
+	break;
 }
 }
 }
namespace std
{
  template < typename _Alloc > class allocator;
  template < class _CharT > struct char_traits;
template < typename _CharT, typename _Traits =
char_traits < _CharT >, typename _Alloc =
allocator < _CharT > >class basic_string;
  typedef basic_string < char >string;
  typedef long unsigned int size_t;
template <> struct char_traits 
  {
typedef char char_type;
static size_t length (const char_type * __s)
{
  return __builtin_strlen (__s);
}
  };
  namespace __gnu_cxx
  {
template < typename _Tp > class new_allocator
{
public:
  typedef size_t size_type;
template < typename _Tp1 > struct rebind
  {
typedef new_allocator < _Tp1 > other;
  };
};
  }
template < typename _Tp > class allocator:public __gnu_cxx::new_allocator <
_Tp >
  {
  };
  template < typename _CharT, typename _Traits, typename _Alloc >
class basic_string
  {
typedef typename _Alloc::template rebind <
  _CharT >::other _CharT_alloc_type;
typedef _Traits traits_type;
typedef typename _CharT_alloc_type::size_type size_type;
  public:
basic_string & operator= (const _CharT * __s)
{
  return this->assign (__s, traits_type::length (__s));
}
basic_string & assign (const _CharT * __s, size_type __n);
  };

  class Regex
  {
std::string sub (std::string * Error);
  };

  std::string Regex::sub (std::string * Error)
  {
*Error = "";
  }
}

Re: [PATCH][RFC] Sanity checking for -freorder-blocks-and-partition failures

[Teresa Johnson]

Ok, thanks. Will do, with appropriate credit. =)
Teresa

On Fri, Nov 9, 2012 at 12:12 PM, Steven Bosscher  wrote:
> Hello Teresa,
>
> It seems to me that it's better if you commit it along with your set
> of fixes. My patch doesn't fix any bugs, it just exposes them :-)
>
> Ciao!
> Steven
>
>
>
> On Fri, Nov 9, 2012 at 9:09 PM, Teresa Johnson  wrote:
>>
>> Hi Steven,
>>
>> I've spent this week trying to clean up all the issues exposed by this new 
>> verification patch. Some of the issues I described in the email thread on my 
>> related patch (http://gcc.gnu.org/ml/gcc-patches/2012-11/msg00287.html) and 
>> earlier in this thread. It also exposed more issues of the type described in 
>> the last message regarding my patch (the link I included here), where 
>> transformations were being applied but the partitions not being correctly 
>> fixed up. Things look clean now across SPEC2006 int C benchmarks at peak, 
>> gcc regression tests and our internal benchmarks. I need to update from 
>> head, retest and clean things up though before sending the new patch. But do 
>> you want to go ahead and commit this patch? I guess it should be fine to 
>> commit asynchronously with mine since -freorder-blocks-and-partition is off 
>> by default and not working anyway. I assume it can still go in since it was 
>> proposed already and is related to some outstanding bugs?
>>
>> Thanks,
>> Teresa
>>
>>
>> On Wed, Oct 31, 2012 at 1:06 PM, Teresa Johnson  wrote:
>>>
>>> On Wed, Oct 31, 2012 at 12:58 PM, Christophe Lyon
>>>  wrote:
>>> > On 30.10.2012 17:59, Teresa Johnson wrote:
>>> >>
>>> >> On Tue, Oct 30, 2012 at 9:26 AM, Steven Bosscher 
>>> >> wrote:
>>> >>>
>>> >>> Hello,
>>> >>>
>>> >>> Hot/cold partitioning is apparently a hot topic all of a sudden, which
>>> >>> is a good thing of course, because it's in need of some TLC.
>>> >>>
>>> >>> The attached patch adds another check the RTL cfg checking
>>> >>> (verify_flow_info) for the partitioning: A hot block can never be
>>> >>> dominated by a cold block (because the dominated block must also be
>>> >>> cold). This trips in PR55121.
>>> >>>
>>> >>> I haven't tested this with any profiling tests, but it's bound to
>>> >>> break things. From my POV, whatever gets broken by this patch was
>>> >>> already broken to begin with :-)   If you're in CC, it's because I
>>> >>> hope you can help test this patch.
>>> >>
>>> >> I will try testing your patch on top of mine with our fdo benchmarks.
>>> >> For the others on the cc list, you may need to include my patch as
>>> >> well for testing. Without it, -freorder-blocks-and-partition was DOA
>>> >> for me. For my patch, see
>>> >> http://gcc.gnu.org/ml/gcc-patches/2012-10/msg02692.html
>>> >>
>>> >> Teresa
>>> >>
>>> > I have tried Steven's patch an indeed it reported numerous errors while
>>> > compiling spec2k.
>>> >
>>> > I tried Teresa's patch too, but it changed nothing in my tests. The 
>>> > patches
>>> > already posted by Matt are still necessary and Teresa's patch does not
>>> > improve my tests.
>>>
>>> With checking enabled I am seeing additional failures that my fixes
>>> are not addressing. Looking into those now.
>>> Can someone point me to Matt's patches? Is that this one:
>>> http://gcc.gnu.org/ml/gcc-patches/2012-09/msg00274.html
>>> or are there others?
>>>
>>> Thanks,
>>> Teresa
>>>
>>> >
>>> > I am out of office at the moment, so it's a little bit inconvenient to
>>> > investigate deeper the reasons for all the errors reported by Steven's
>>> > patch. Anyway it looks like it's really needed :)
>>> > I also noticed that some compilations failed with an ICE in calc_dfs_tree 
>>> > at
>>> > dominance.c:395.
>>> >
>>> >
>>> > Christophe.
>>> >
>>>
>>>
>>>
>>> --
>>> Teresa Johnson | Software Engineer | tejohn...@google.com | 408-460-2413
>>
>>
>>
>>
>> --
>> Teresa Johnson | Software Engineer |  tejohn...@google.com |  408-460-2413
>>



-- 
Teresa Johnson | Software Engineer | tejohn...@google.com | 408-460-2413

Re: [PATCH][RFC] Sanity checking for -freorder-blocks-and-partition failures

[Steven Bosscher]

Hello Teresa,

It seems to me that it's better if you commit it along with your set
of fixes. My patch doesn't fix any bugs, it just exposes them :-)

Ciao!
Steven



On Fri, Nov 9, 2012 at 9:09 PM, Teresa Johnson  wrote:
>
> Hi Steven,
>
> I've spent this week trying to clean up all the issues exposed by this new 
> verification patch. Some of the issues I described in the email thread on my 
> related patch (http://gcc.gnu.org/ml/gcc-patches/2012-11/msg00287.html) and 
> earlier in this thread. It also exposed more issues of the type described in 
> the last message regarding my patch (the link I included here), where 
> transformations were being applied but the partitions not being correctly 
> fixed up. Things look clean now across SPEC2006 int C benchmarks at peak, gcc 
> regression tests and our internal benchmarks. I need to update from head, 
> retest and clean things up though before sending the new patch. But do you 
> want to go ahead and commit this patch? I guess it should be fine to commit 
> asynchronously with mine since -freorder-blocks-and-partition is off by 
> default and not working anyway. I assume it can still go in since it was 
> proposed already and is related to some outstanding bugs?
>
> Thanks,
> Teresa
>
>
> On Wed, Oct 31, 2012 at 1:06 PM, Teresa Johnson  wrote:
>>
>> On Wed, Oct 31, 2012 at 12:58 PM, Christophe Lyon
>>  wrote:
>> > On 30.10.2012 17:59, Teresa Johnson wrote:
>> >>
>> >> On Tue, Oct 30, 2012 at 9:26 AM, Steven Bosscher 
>> >> wrote:
>> >>>
>> >>> Hello,
>> >>>
>> >>> Hot/cold partitioning is apparently a hot topic all of a sudden, which
>> >>> is a good thing of course, because it's in need of some TLC.
>> >>>
>> >>> The attached patch adds another check the RTL cfg checking
>> >>> (verify_flow_info) for the partitioning: A hot block can never be
>> >>> dominated by a cold block (because the dominated block must also be
>> >>> cold). This trips in PR55121.
>> >>>
>> >>> I haven't tested this with any profiling tests, but it's bound to
>> >>> break things. From my POV, whatever gets broken by this patch was
>> >>> already broken to begin with :-)   If you're in CC, it's because I
>> >>> hope you can help test this patch.
>> >>
>> >> I will try testing your patch on top of mine with our fdo benchmarks.
>> >> For the others on the cc list, you may need to include my patch as
>> >> well for testing. Without it, -freorder-blocks-and-partition was DOA
>> >> for me. For my patch, see
>> >> http://gcc.gnu.org/ml/gcc-patches/2012-10/msg02692.html
>> >>
>> >> Teresa
>> >>
>> > I have tried Steven's patch an indeed it reported numerous errors while
>> > compiling spec2k.
>> >
>> > I tried Teresa's patch too, but it changed nothing in my tests. The patches
>> > already posted by Matt are still necessary and Teresa's patch does not
>> > improve my tests.
>>
>> With checking enabled I am seeing additional failures that my fixes
>> are not addressing. Looking into those now.
>> Can someone point me to Matt's patches? Is that this one:
>> http://gcc.gnu.org/ml/gcc-patches/2012-09/msg00274.html
>> or are there others?
>>
>> Thanks,
>> Teresa
>>
>> >
>> > I am out of office at the moment, so it's a little bit inconvenient to
>> > investigate deeper the reasons for all the errors reported by Steven's
>> > patch. Anyway it looks like it's really needed :)
>> > I also noticed that some compilations failed with an ICE in calc_dfs_tree 
>> > at
>> > dominance.c:395.
>> >
>> >
>> > Christophe.
>> >
>>
>>
>>
>> --
>> Teresa Johnson | Software Engineer | tejohn...@google.com | 408-460-2413
>
>
>
>
> --
> Teresa Johnson | Software Engineer |  tejohn...@google.com |  408-460-2413
>

Re: [PATCH] Fix array access beyond bounds in test cases

[Jeff Law]

On 11/09/2012 12:14 PM, Siddhesh Poyarekar wrote:

On Fri, 09 Nov 2012 11:57:44 -0700, Jeff wrote:

The off-by-one aspects of 31227 ought to be corrected.

Ok for the trunk,


I don't have write access (not sure if I qualify for it yet with just a
couple of trivial patches so far) so I need someone to commit for me.

OK.  I took care of committing for you.

jeff

Re: Asan/Tsan Unit/Regression testing (was [asan] Emit GIMPLE direclty, small cleanups)

[Jakub Jelinek]

On Fri, Nov 09, 2012 at 08:13:12PM +0100, Jakub Jelinek wrote:
> On Fri, Nov 09, 2012 at 11:05:37AM -0800, Wei Mi wrote:
> > gtest integrate multiple tests into the same file with each test being
> > a single line check.  I cannot think out a method to migrate it to
> > dejagnu without using gtest, except splitting a single gtest file to
> > multiple files with each file per test. asan has about 130 tests so
> > have to write 130 files which will be a doable but painful task.
> 
> See the glibc _FORTIFY_SOURCE check I've referenced, there it is 3 lines
> per test expected to crash, but could be done in a single macro too.
> If the failure can be intercepted, it can be done in a single process (e.g.
> SIGABRT can, _exit can't that easily), otherwise perhaps say way to skip
> previous tests and communicate with dejagnu how many times it should run the
> executable.

And quick grep shows that one can call __sanitizer::SetDieCallback
and supposedly, if ASAN doesn't hold any locks when it calls Die() that
would prevent it from functioning further, we can just longjmp out from the
callback.

Jakub

Re: [PATCH] Fix array access beyond bounds in test cases

[Siddhesh Poyarekar]

On Fri, 09 Nov 2012 11:57:44 -0700, Jeff wrote:
> The off-by-one aspects of 31227 ought to be corrected.
> 
> Ok for the trunk,

I don't have write access (not sure if I qualify for it yet with just a
couple of trivial patches so far) so I need someone to commit for me.

Thanks,
Siddhesh

Re: Asan/Tsan Unit/Regression testing (was [asan] Emit GIMPLE direclty, small cleanups)

[Jakub Jelinek]

On Fri, Nov 09, 2012 at 11:05:37AM -0800, Wei Mi wrote:
> gtest integrate multiple tests into the same file with each test being
> a single line check.  I cannot think out a method to migrate it to
> dejagnu without using gtest, except splitting a single gtest file to
> multiple files with each file per test. asan has about 130 tests so
> have to write 130 files which will be a doable but painful task.

See the glibc _FORTIFY_SOURCE check I've referenced, there it is 3 lines
per test expected to crash, but could be done in a single macro too.
If the failure can be intercepted, it can be done in a single process (e.g.
SIGABRT can, _exit can't that easily), otherwise perhaps say way to skip
previous tests and communicate with dejagnu how many times it should run the
executable.

Jakub

Re: Asan/Tsan Unit/Regression testing (was [asan] Emit GIMPLE direclty, small cleanups)

[Wei Mi]

>
>>
>> > 2. Large Gtest-based unittest. This is a set of c++ files that should
>> > be built with the asan switch, depends on gtest
>> > (http://code.google.com/p/googletest/).
>> >
>> > http://llvm.org/viewvc/llvm-project/compiler-rt/trunk/lib/asan/tests/asan_test.cc?revision=166104&view=markup
>> > This should be easy to port to GCC, but it requires gtest.
>>
>> I don't think we want to depend on gtest, if the tests only use a small
>> subset of that, it would be better to just use some header with macros
>> compatible with that for assertions and the like.
>
>
> We use a large and heavy subset of gtest, namely: death tests.
> It can't be easily replaced with "some header with macros".
>

gtest integrate multiple tests into the same file with each test being
a single line check.  I cannot think out a method to migrate it to
dejagnu without using gtest, except splitting a single gtest file to
multiple files with each file per test. asan has about 130 tests so
have to write 130 files which will be a doable but painful task.

>>
>>
>> > 3. Full output tests (a .cc file should be build with asan switch,
>> > executable should be run and the stderr is compared with the expected
>> > output)
>> > Example:
>> > http://llvm.org/viewvc/llvm-project/compiler-rt/trunk/lib/asan/lit_tests/stack-overflow.cc?revision=165391&view=markup
>> > The can be ported to GCC, but the uses of FileCheck
>> > (http://llvm.org/docs/CommandGuide/FileCheck.html) will need to be
>> > replaced with GCC's analog.
>> > We should probably start with these tests.
>>
>> Dejagnu in GCC has
>>
>> ! { dg-do run }
>> ! { dg-options "-fbounds-check" }
>> ! { dg-shouldfail "Duplicate value 2 in ORDER argument to RESHAPE
>> intrinsic" }
>> program main
>>   implicit none
>>   integer(kind=1), dimension(6) :: source1 = (/ 1, 2, 3, 4, 5, 6 /)
>>   integer, dimension(2) :: shape1 = (/ 2, 3/)
>>   integer(kind=1), dimension(2) :: pad1 = (/ 0, 0/)
>>   character(len=200) :: l1, l2
>>   integer :: i1, i2
>>
>>   l1 = "2 2"
>>   read(unit=l1,fmt=*) i1, i2
>>   write (unit=l2,fmt=*) reshape(source1, shape1, pad1, (/i1, i2/)) !
>> Invalid
>> end program main
>> ! { dg-output "Fortran runtime error: Duplicate value 2 in ORDER argument
>> to RESHAPE intrinsic" }
>>
>> style markings, dg-shouldfail says that the program is expected to fail
>> rather than pass (if it aborts), and dg-output (perhaps multiple) can
>> contain regexps to match against stderr + stdout joined.  Haven't looked
>> at the asan tests yet, do you expect just one ASAN abort per test,
>
>
> These tests do just one abort (actually, _exit(1)) per test.
> Let's start with these.
>
> --kcc
>

I will start with this.

Thanks,
Wei.

Re: [PATCH] Fix array access beyond bounds in test cases

[Jeff Law]

On 11/09/2012 12:12 AM, Siddhesh Poyarekar wrote:

Hi,

I found that some test cases access arrays beyond their bounds.  I
looked up their originating bugzillas and found that the test
cases for pr22506 and pr34005 were likely to be typos since the
original test cases in the report do not have this problem.  For
pr31227 however, I am inclined to think that the test case is
incorrect.  I am not sure what the test case verifies since the array
accesses are obviously beyond bounds.

Regards,
Siddhesh

testsuite/ChangeLog:

2012-11-09  Siddhesh Poyarekar  

* gcc.dg/Warray-bounds-3.c (bar): Keep array access within
bounds for ABDAY, DAY, ABMON, MON, AM_PM.
* gcc.dg/vect/pr22506.c (foo): Reduce loop iterations to within
array bounds.
* gcc.dg/vect/pr34005.c (XdmcpUnwrap): Likewise.
For 31227 it's checking that an address formed by first computing an 
address outside the array, then adding an offset with the end result 
pointing at a valid location inside the array doesn't generate an 
incorrect warning.


This style of address computation can sometimes be advantageous for the 
loop optimizers to generate (my recollection is it's invalid C/C++ at 
the source level).


The off-by-one aspects of 31227 ought to be corrected.

Ok for the trunk,
jeff

Re: [C++11] PR54413 Option for turning off compiler extensions for numeric literals.

[Jason Merrill]

On 11/08/2012 11:55 PM, Ed Smith-Rowland wrote:

+literal number suffixes as Gnu extensions.


GNU is all-caps.  OK with that change.

Jason

[committed] fix g++.dg/tm/pr47530-2.C and pr47530.C

[Aldy Hernandez]

Now that we have two code paths, the testsuite needs to be adjusted.

Committed as obvious.
commit 1cfee58f7559592cc21e8c68754a1c0093c5876d
Author: Aldy Hernandez 
Date:   Fri Nov 9 12:42:00 2012 -0600

+   * g++.dg/tm/pr47530-2.C: Adjust for uninstrumented code path.
+   * g++.dg/tm/pr47530.C: Same.

diff --git a/gcc/testsuite/ChangeLog b/gcc/testsuite/ChangeLog
index e888313..fa0919e 100644
--- a/gcc/testsuite/ChangeLog
+++ b/gcc/testsuite/ChangeLog
@@ -1,3 +1,8 @@
+2012-11-09  Aldy Hernandez  
+
+   * g++.dg/tm/pr47530-2.C: Adjust for uninstrumented code path.
+   * g++.dg/tm/pr47530.C: Same.
+
 2012-11-09  Jason Merrill  
 
PR c++/54859
diff --git a/gcc/testsuite/g++.dg/tm/pr47530-2.C 
b/gcc/testsuite/g++.dg/tm/pr47530-2.C
index c98e07e..bcfdbe9 100644
--- a/gcc/testsuite/g++.dg/tm/pr47530-2.C
+++ b/gcc/testsuite/g++.dg/tm/pr47530-2.C
@@ -31,5 +31,9 @@ void bench_test()
 }
 }
 
-// { dg-final { scan-tree-dump-times "ITM_commitTransaction.*tail call" 0 
"tmedge" } }
+// There should be two calls to commitTransaction.
+// The one in the uninstrumented code path is a tail call.
+// The one in the instrumented code path is not.
+// { dg-final { scan-tree-dump-times "ITM_commitTransaction.*tail call" 1 
"tmedge" } }
+
 // { dg-final { cleanup-tree-dump "tmedge" } }
diff --git a/gcc/testsuite/g++.dg/tm/pr47530.C 
b/gcc/testsuite/g++.dg/tm/pr47530.C
index 8e7e272..6a9fb1b 100644
--- a/gcc/testsuite/g++.dg/tm/pr47530.C
+++ b/gcc/testsuite/g++.dg/tm/pr47530.C
@@ -73,7 +73,8 @@ void LinkedList::insert(int val)
   }
 }
 
-// Make sure we don't do tail optimization on the commit.
-// { dg-final { scan-tree-dump-times "commitTransaction...; .tail call" 0 
"optimized" } }
+// Make sure we don't do tail optimization on the commit, except on
+// the uninstrumented code path.
+// { dg-final { scan-tree-dump-times "commitTransaction...; .tail call" 1 
"optimized" } }
 // { dg-final { cleanup-tree-dump "optimized" } }
 

Re: [PATCH] Handle abortTransaction with RTM

[Torvald Riegel]

On Fri, 2012-11-09 at 10:24 -0800, Andi Kleen wrote:
> Torvald Riegel  writes:
> 
> > On Thu, 2012-11-08 at 17:23 -0800, Richard Henderson wrote:
> >> + // Honor an abort from abortTransaction.
> >> + else if (htm_abort_is_cancel(ret))
> >> +   return a_abortTransaction | a_restoreLiveVariables;
> >
> > The problem is that we cannot reliably detect whether an abort with a
> > certain abort reason code really means that we got canceled.  A nested
> > transaction is one example: how do we distinguish whether the nested or
> > the outermost transaction were canceled?.
> 
> In TSX you always come back to the outermost anyways.

That, and the fact that we cannot reliably communicate which transaction
(on the programming language level) was supposed to be aborted, is
exactly the problem that I described.

Re: [PATCH] Handle abortTransaction with RTM

[Andi Kleen]

Torvald Riegel  writes:

> On Thu, 2012-11-08 at 17:23 -0800, Richard Henderson wrote:
>> + // Honor an abort from abortTransaction.
>> + else if (htm_abort_is_cancel(ret))
>> +   return a_abortTransaction | a_restoreLiveVariables;
>
> The problem is that we cannot reliably detect whether an abort with a
> certain abort reason code really means that we got canceled.  A nested
> transaction is one example: how do we distinguish whether the nested or
> the outermost transaction were canceled?.

In TSX you always come back to the outermost anyways.
Look at the nested bit in the abort code?

> Thus, there are two options for how to handle transactions that may
> abort: Either execute them transactionally the first time, and do an
> explicit HTM abort on transaction_cancel that tells libitm not not retry
> as HW transaction, or execute such transactions always as SW
> transactions.  Which of these two options is better depends on how
> frequently transaction_cancel would actually be called.  If it's
> infrequent, then trying to run as HW transactions might be better.

I believe the current evidence is that cancel is very uncommon.

-Andi

-- 
a...@linux.intel.com -- Speaking for myself only

Re: [PATCH] Handle abortTransaction with RTM

[Andi Kleen]

Torvald Riegel  writes:
>
> I'm not sure this is quite true.  If a libitm-executed transaction is
> started from within some other transactional region (e.g., managed
> explicitly by the user), and those two disagree about what is an abort
> that should be retried or not, then this can at least have a negative
> impact on performance: Either because the outermost HW transaction
> handler thinks it shouldn't retry even though it should, or because the
> outermost transaction retries continuously even though there's no way it
> can succeed.

It shouldn't retry continuously for a nested abort.
Also retrying a lot is dangerous anyways, you have to be very
careful about that.

Short term your biggest problem is profiling the whole thing in any
case, and that is what the XABORT with well defined codes make much
easier.

-Andi


-- 
a...@linux.intel.com -- Speaking for myself only

Re: [PATCH] Handle abortTransaction with RTM

[Torvald Riegel]

On Thu, 2012-11-08 at 20:34 -0800, Andi Kleen wrote:
> Richard Henderson  writes:
> >  
> >  static inline void
> > -htm_abort ()
> > +htm_abort_retry ()
> >  {
> >// ??? According to a yet unpublished ABI rule, 0xff is reserved and
> >// supposed to signal a busy lock.  Source: andi.kl...@intel.com
> >_xabort(0xff);
> >  }
> >  
> > +static inline void
> > +htm_abort_cancel ()
> > +{
> > +  // ??? What's the unpublished ABI rule for this, Andi?
> 
> There is none for cancel, just for lock-is-locked (0xfe) and lock-busy (0xff)
> 
> The convention is just for easier abort profiling. The profiler (perf) can 
> display
> this abort code and it's far easier to understand if common situations
> have their standard code. But you can always make up your own too.

I'm not sure this is quite true.  If a libitm-executed transaction is
started from within some other transactional region (e.g., managed
explicitly by the user), and those two disagree about what is an abort
that should be retried or not, then this can at least have a negative
impact on performance: Either because the outermost HW transaction
handler thinks it shouldn't retry even though it should, or because the
outermost transaction retries continuously even though there's no way it
can succeed.

If all code that uses transactions will eventually choose the fallback
path, this is not a correctness issue.  But it can be a performance
issue, so I think that HTMs and the code using them should be careful to
agree about abort reasons and what they communicate.


Torvald

Re: [PATCH] Handle abortTransaction with RTM

[Torvald Riegel]

On Thu, 2012-11-08 at 17:23 -0800, Richard Henderson wrote:
> + // Honor an abort from abortTransaction.
> + else if (htm_abort_is_cancel(ret))
> +   return a_abortTransaction | a_restoreLiveVariables;

The problem is that we cannot reliably detect whether an abort with a
certain abort reason code really means that we got canceled.  A nested
transaction is one example: how do we distinguish whether the nested or
the outermost transaction were canceled?.
Another example is transaction_pure or tm_wrapper code that might reuse
the same abort reason for explicit aborts: we would like to allow
malloc() calls in transactions, but if malloc uses transactions with
explicit aborts, we might abort a transaction without a cause.

It might be possible to avoid that by carefully specifying the rules for
abort reasons, which of those can be used in nested transactions, and so
on.  But I'm not aware of any specification like that, and it seems to
likely be a pretty fragile approach.

Thus, there are two options for how to handle transactions that may
abort: Either execute them transactionally the first time, and do an
explicit HTM abort on transaction_cancel that tells libitm not not retry
as HW transaction, or execute such transactions always as SW
transactions.  Which of these two options is better depends on how
frequently transaction_cancel would actually be called.  If it's
infrequent, then trying to run as HW transactions might be better.

Torvald

Re: [Darwin] Do not enable -fvar-tracking at -O0

[Eric Botcazou]

>Is this a hack around the underlying problem on darwin or the
> implementation of the same approach used on other targets? If the former,
> shouldn't we have a PR opened to properly fix this in the long term?

Neither, it's a fix and other targets don't care about -fvar-tracking-uninit.

-- 
Eric Botcazou

Re: Patch: Print SImode register names to force addr32 prefix

[Uros Bizjak]

On Fri, Nov 9, 2012 at 10:17 AM, H.J. Lu  wrote:

> Since x32 runs in 64-bit mode, for address -0x4300(%rax), hardware
> sign-extends displacement from 32-bits to 64-bits and adds it to %rax.
> But x32 wants 32-bit -0x4300, not 64-bit -0x4300.  This patch
> uses 32-bit registers instead of 64-bit registers when displacement
> < -16*1024*1024.  -16*1024*1024 is used instead of 0 so that we will
> still generate -16(%rsp) instead of -16(%esp).
>
> Tested it on Linux/x32.  OK to install?

This problem uncovers a bug in the middle-end, so I guess it would be
better to fix it there.

Uros.

Re: [PATCH] PR tree-optimization/55079: Don't remove all exits of a loop during loop unroll

[Siddhesh Poyarekar]

On Fri, 9 Nov 2012 17:34:26 +0100, Jan wrote:
> > I don't mind saying that GCC should define cases that the language
> > standards leave undefined.  But that does not seem to be what this
> > patch does.  I don't see why this is a good idea.  It seems to
> > produce a program that is unpredictable in a different way.  How
> > does that help the user?  If anything an infinite loop might be
> > better, since it is more obvious that something is wrong.  Unless I
> > misunderstand.
> 
> I think resonable thing to do is to output warning in this case
> "Loop reaches undefined effect before any of the exit conditions are
> satisfied; turned into infinite loop" or something along these
> lines?  We can probably even get the location info of the statement
> that implied the loop bound.

I had reckoned that the behaviour could be reverted to what was before
while I figure out a way to get the warning in place for both cases,
i.e. with tree-vrp (where max_loop_iterations now causes the loop to be
folded away in -O2) and this unroll case (in -O1).  I'll look at
getting a warning for the tree-vrp case separately if the infinite loop
behaviour needs to be retained.

An infinite loop without the warning breaks diagnostic apps like
valgrind since they are no longer in a position to detect this.  The
user would then have to somehow conclude that their loop loops
infinitely because they have a beyond-array-bounds access in it.
However, the infinite loop could be OK if there is an explicit warning
telling the user that it's going to happen.  I already have some rough
code in place to do the warning against the loop header, so I'll clean
it up to issue a warning for all exit points beyond the upper bound
when we see that no exits remain.  It will break a few existing test
cases though - I had posted a patch to fix those earlier today:

http://gcc.gnu.org/ml/gcc-patches/2012-11/msg00722.html

Thanks,
Siddhesh

[i386] Tidy TARGET_LRA_P

[Richard Henderson]

Until and unless we need to do something special in the hook,
use a generic function.  Change this before other targets end
up duplicating the mistake.

Committed.


r~
* config/i386/i386.c (ix86_lra_p): Remove.
(TARGET_LRA_P): Use hook_bool_void_true.


diff --git a/gcc/config/i386/i386.c b/gcc/config/i386/i386.c
index 3a6f494..60a39b8 100644
--- a/gcc/config/i386/i386.c
+++ b/gcc/config/i386/i386.c
@@ -32856,13 +32856,6 @@ ix86_free_from_memory (enum machine_mode mode)
 }
 }
 
-/* Return true if we use LRA instead of reload pass.  */
-static bool
-ix86_lra_p (void)
-{
-  return true;
-}
-
 /* Return a register priority for hard reg REGNO.  */
 static int
 ix86_register_priority (int hard_regno)
@@ -42119,7 +42112,7 @@ ix86_memmodel_check (unsigned HOST_WIDE_INT val)
 #define TARGET_LEGITIMATE_ADDRESS_P ix86_legitimate_address_p
 
 #undef TARGET_LRA_P
-#define TARGET_LRA_P ix86_lra_p
+#define TARGET_LRA_P hook_bool_void_true
 
 #undef TARGET_REGISTER_PRIORITY
 #define TARGET_REGISTER_PRIORITY ix86_register_priority

Re: [PATCH 01/10] Initial import of asan from the Google branch into trunk

[Wei Mi]

> Other issues:
>
> * libasan does not seem to be a multilib, at least I only find the 64bit
> version on x86-64-gnu-linux such that "-m32" compilation fails.
>

That is because originally configure file is shared between asan and
tsan (tsan doesn't support 32 bit). Diego has suggested me to split
the configure, so we will send a patch to support 32bit version asan
after Dodji's patches checkin to trunk.

Thanks,
Wei.

Re: [PATCH 01/10] Initial import of asan from the Google branch into trunk

[Tobias Burnus]

Jakub Jelinek wrote:

On Fri, Nov 09, 2012 at 02:14:04PM +0100, Tobias Burnus wrote:

I tried the 01/10 to 10/10 patch series but it doesn't trigger for
the following test case:

[...]

Can't reproduce that (admittedly with asan SVN branch rather than the patchset):


I can reproduce both; comparing the asan0 dumps and the two asan.c 
files, I came up with the following patch. If I apply it on top of 
Dodji's branch, it now also aborts.



--- a/gcc/asan.c
+++ b/gcc/asan.c
@@ -1536,10 +1536,10 @@ static void
asan_init_shadow_ptr_types (void)
{
asan_shadow_set = new_alias_set ();
- shadow_ptr_types[0] = build_distinct_type_copy (unsigned_char_type_node);
+ shadow_ptr_types[0] = build_distinct_type_copy (signed_char_type_node);
TYPE_ALIAS_SET (shadow_ptr_types[0]) = asan_shadow_set;
shadow_ptr_types[0] = build_pointer_type (shadow_ptr_types[0]);
- shadow_ptr_types[1] = build_distinct_type_copy (short_unsigned_type_node);
+ shadow_ptr_types[1] = build_distinct_type_copy (short_integer_type_node);
TYPE_ALIAS_SET (shadow_ptr_types[1]) = asan_shadow_set;
shadow_ptr_types[1] = build_pointer_type (shadow_ptr_types[1]);
}



Other remarks:

* libsanitizer: It currently installs under "lib" even under a 
x86-64-gnu-linux system where it should be "lib64"; that's probably 
automatically fix by enabling the multilib support. Maybe, removing the 
"#" before "#AM_ENABLE_MULTILIB" in libsanitizer/configure.ac is sufficient


* invoke.texi: The fbranch-target-load-optimize2 and fauto-inc-dec lost 
the "-", additionally, there are several "--f…" items.


Tobias

PS: Has someone an idea why I cannot run the -faddress-sanitizer 
executable on gcc20 of the GCC compile farm? Fails with the following 
message, but "ulimit -v" is set to "unlimited".


==4784== ReserveShadowMemoryRange failed while trying to map 
0x2001000 bytes. Perhaps you're using ulimit -v

Re: [Patch] libitm: add HTM fastpath

[Torvald Riegel]

On Tue, 2012-11-06 at 08:11 -0800, Richard Henderson wrote:
> On 2012-11-05 17:09, Torvald Riegel wrote:
> > +  if (likely(htm_fastpath && (prop & pr_hasNoAbort)))
> 
> For reference, could the NoAbort clause be relaxed with an htm check
> in abortTransaction, and the use of an xabort insn with an appropriate
> code to indicate user abort?

We have been discussing this off-line, and might have a patch for this
at a later point in time.

For now, I've committed the patch with Uros change applied.

Re: [PATCH] Handle abortTransaction with RTM

[Richard Henderson]

On 2012-11-08 17:23, Richard Henderson wrote:
> I believe this is the sort of patch that Torvald was talking about
> for handling abortTransaction with RTM.

FYI, I realized that this patch doesn't handle aborts on a
nested transaction properly.


r~

Re: [PATCH] Handle target specific memory models in C frontend

[Andi Kleen]

Richard Henderson  writes:

> On 2012-11-09 07:18, Jakub Jelinek wrote:
>> I think it actually should be removed, given that e.g.
>> int i, j;
>> void
>> foo (void)
>> {
>>   __atomic_store (&i, &j, 12);
>> }
>> warns twice for the same thing, when it should only warn once.
>> Richard?
>
> Makes sense.

Ok, I'll remove it completely and fix the ChangeLog.

Thanks.

-Andi

-- 
a...@linux.intel.com -- Speaking for myself only

Re: [PATCH 01/10] Initial import of asan from the Google branch into trunk

[Xinliang David Li]

It seems that my one line fix in asan branch (r192605) is not included
in Dodji's patch set.

David


On Fri, Nov 9, 2012 at 5:58 AM, Jakub Jelinek  wrote:
> On Fri, Nov 09, 2012 at 02:14:04PM +0100, Tobias Burnus wrote:
>> Dodji Seketeli wrote:
>> >This patch imports the initial state of asan as it was in the
>> >Google branch.
>> >
>> >It provides basic infrastructure for asan to instrument memory
>> >accesses on the heap, at -O3.  Note that it supports neither stack nor
>> >global variable protection.
>>
>> I tried the 01/10 to 10/10 patch series but it doesn't trigger for
>> the following test case:
>>
>> #include 
>> #include 
>>
>> int
>> main() {
>>   int *i;
>>   i = malloc(10*sizeof(*i));
>>   free(i);  /* <<< Free memory. */
>>   i[10] = 5;  /* <<< out of boundary even if not freed. */
>>   printf("%d\n", i[11]);  /* <<< out of boundary even if not freed. */
>>   return 0;
>> }
>>
>> (All of them are reported by Clang.) If I look at the dump (or
>> assembler), I see the call to __asan_init, __asan_report_store4 and
>> __asan_report_load4. However, when running the program ltrace only
>> shows the calls to: __libc_start_main, __asan_init, malloc, free and
>> printf. I haven't debugged why the condition is false [see
>> attachment for the dump].
>
> Can't reproduce that (admittedly with asan SVN branch rather than the
> patchset):
>
> ./xgcc -B ./ -O2 -fasan -o a a.c 
> -Wl,-rpath,/usr/src/gcc-asan/obj/x86_64-unknown-linux-gnu/libsanitizer/asan/.libs/
>  \
>  -L 
> /usr/src/gcc-asan/obj/x86_64-unknown-linux-gnu/libsanitizer/asan/.libs/
> ./a
> =
> ==20614== ERROR: AddressSanitizer heap-use-after-free on address
> 0x7f7d8245afec at pc 0x4006f8 bp 0x7fff9beda4c0 sp 0x7fff9beda4b8
> READ of size 4 at 0x7f7d8245afec thread T0
> #0 0x4006f7 (/usr/src/gcc-asan/obj/gcc/a+0x4006f7)
> 0x7f7d8245afec is located 4 bytes to the right of 40-byte region
> [0x7f7d8245afc0,0x7f7d8245afe8)
> freed by thread T0 here:
> #0 0x7f7d82796585
> 
> #(/usr/src/gcc-asan/obj/x86_64-unknown-linux-gnu/libsanitizer/asan/.libs/libasan.so.0.0.0+0xf585)
> #1 0x4006b5 (/usr/src/gcc-asan/obj/gcc/a+0x4006b5)
> previously allocated by thread T0 here:
> #0 0x7f7d82796645
> 
> #(/usr/src/gcc-asan/obj/x86_64-unknown-linux-gnu/libsanitizer/asan/.libs/libasan.so.0.0.0+0xf645)
> #1 0x4006aa (/usr/src/gcc-asan/obj/gcc/a+0x4006aa)
> Shadow byte and word:
>   0x1fefb048b5fd: fd
>   0x1fefb048b5f8: fd fd fd fd fd fd fd fd
> More shadow bytes:
>   0x1fefb048b5d8: fa fa fa fa fa fa fa fa
>   0x1fefb048b5e0: fa fa fa fa fa fa fa fa
>   0x1fefb048b5e8: fa fa fa fa fa fa fa fa
>   0x1fefb048b5f0: fa fa fa fa fa fa fa fa
> =>0x1fefb048b5f8: fd fd fd fd fd fd fd fd
>   0x1fefb048b600: fa fa fa fa fa fa fa fa
>   0x1fefb048b608: fa fa fa fa fa fa fa fa
>   0x1fefb048b610: fa fa fa fa fa fa fa fa
>   0x1fefb048b618: fa fa fa fa fa fa fa fa
> Stats: 0M malloced (0M for red zones) by 1 calls
> Stats: 0M realloced by 0 calls
> Stats: 0M freed by 1 calls
> Stats: 0M really freed by 0 calls
> Stats: 0M (128 full pages) mmaped in 1 calls
>   mmaps   by size class: 7:4095;
>   mallocs by size class: 7:1;
>   frees   by size class: 7:1;
>   rfrees  by size class:
> Stats: malloc large: 0 small slow: 1
> ==20614== ABORTING
>
> Jakub

Re: [PATCH] Handle target specific memory models in C frontend

[Richard Henderson]

On 2012-11-09 07:18, Jakub Jelinek wrote:
> I think it actually should be removed, given that e.g.
> int i, j;
> void
> foo (void)
> {
>   __atomic_store (&i, &j, 12);
> }
> warns twice for the same thing, when it should only warn once.
> Richard?

Makes sense.


r~

RE: [RFC] New feature to reuse one multilib among different targets

[Joseph S. Myers]

On Fri, 9 Nov 2012, Terry Guo wrote:

> You are right that we should make script POSIX compliant. This v3 patch
> removed "function" and "local" which don't belong to POSIX standard. I also
> verified that CONFIG_SHELL is passed to this script with value "/bin/sh".

Suppose /bin/sh is not a POSIX shell but the user sets CONFIG_SHELL to 
something else (which is a POSIX shell).  Will SHELL in the makefile get 
set to the POSIX shell the user specified as CONFIG_SHELL?  That's what's 
needed to be able to use POSIX shell features in this script.

-- 
Joseph S. Myers
jos...@codesourcery.com

Re: [PATCH] PR tree-optimization/55079: Don't remove all exits of a loop during loop unroll

[Jan Hubicka]

> On Fri, Nov 9, 2012 at 5:11 AM, Siddhesh Poyarekar  
> wrote:
> >
> > I understand that the behaviour is undefined, but this is easily
> > avoidable by skipping removal of the exits if it results in an infinite
> > loop.  Attached patch does exactly that.
> 
> I don't mind saying that GCC should define cases that the language
> standards leave undefined.  But that does not seem to be what this
> patch does.  I don't see why this is a good idea.  It seems to produce
> a program that is unpredictable in a different way.  How does that
> help the user?  If anything an infinite loop might be better, since it
> is more obvious that something is wrong.  Unless I misunderstand.

I think resonable thing to do is to output warning in this case
"Loop reaches undefined effect before any of the exit conditions are satisfied; 
turned into infinite loop"
or something along these lines?  We can probably even get the location info of
the statement that implied the loop bound.

This is particularly simple case where undefined code is now handled
differently.  What I am more worried about is the case where we wire in
__builtin_unreachable that will stay reachable and the resulting program will
die in difficult to debug way.  I was thinking about adding similar warning
when __builtin_unreachable starts to be unconditional.

Honza

Re: [PATCH] Attribute for unused warning for variables of non-trivial types

[Gabriel Dos Reis]

On Fri, Nov 9, 2012 at 5:08 AM, Florian Weimer  wrote:
> On 11/08/2012 08:43 PM, Lubos Lunak wrote:
>
>>   The patch implements an attribute for marking types for which gcc cannot
>> on
>> its own issue warnings about unused variables (e.g. because the ctor is
>> external), but for which such a warning might be useful anyway (e.g.
>> std::string).
>
>
> I'm not sure if the default shouldn't be "warn".  RAII-only classes which
> are used purely for constructor/destructor side effects are pretty rare,
> AFAICT.
>
> To make this useful with containers, we'd need further annotations to tell
> read and write accesses apart.  (A vector might be append-only, and the
> elements might be ignored.)
>
> --
> Florian Weimer / Red Hat Product Security Team

I am also concerned that an idiomatic standard C++ program would need
obscure annotations to silence GCC.

-- Gaby

patch to fix PR55154

[Vladimir Makarov]

  The following patch fixes

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=55154

  The insn in question has 2 alternatives.  The first alternative is 
rejected because operand constraint is class containing non-allocated 
hard reg for given options.  The second alternative was rejected by LRA 
as the maximal cost was achieved. Reload sets up to close to maximal 
cost when it sees ! and this is the last operand (imho setting to max 
reject in reload is wrong as if ! were in the 1st operand, reload would 
also reject the alternative).  LRA accumulates the costs and with 
reloads of previous operands it easily achieved maximal cost.  The patch 
affects a sensible code of LRA and therefore was rigorously tested.


  The patch was successfully bootstrapped and tested on x86/x86-64.

  Committed as rev. 193364.

2012-11-09  Vladimir Makarov  

PR tree-optimization/55154
* lra-int.h (LRA_LOSER_COST_FACTOR, LRA_MAX_REJECT): New macros.
* lra.c (setup_operand_alternative): Use them.
* lra-constraints.c (LOSER_COST_FACTOR, MAX_OVERALL_COST_BOUND):
Remove.
(process_alt_operands): Use LRA_LOSER_COST_FACTOR and
LRA_MAX_REJECT.  Accumulate reject instead of setting for
non-const.
(curr_insn_transform): Initialize best_losers and best_overall by
INT_MAX.

2012-11-09  Vladimir Makarov 

PR rtl-optimization/55154
* gcc.target/i386/pr55154.c: New test.

Index: lra.c
===
--- lra.c   (revision 193349)
+++ lra.c   (working copy)
@@ -784,10 +784,10 @@ setup_operand_alternative (lra_insn_reco
  break;
 
case '?':
- op_alt->reject += 6;
+ op_alt->reject += LRA_LOSER_COST_FACTOR;
  break;
case '!':
- op_alt->reject += 600;
+ op_alt->reject += LRA_MAX_REJECT;
  break;
case '&':
  op_alt->earlyclobber = 1;
Index: lra-constraints.c
===
--- lra-constraints.c   (revision 193349)
+++ lra-constraints.c   (working copy)
@@ -1282,12 +1282,6 @@ general_constant_p (rtx x)
   return CONSTANT_P (x) && (! flag_pic || LEGITIMATE_PIC_OPERAND_P (x));
 }
 
-/* Cost factor for each additional reload and maximal cost bound for
-   insn reloads.  One might ask about such strange numbers.  Their
-   values occurred historically from former reload pass.  */
-#define LOSER_COST_FACTOR 6
-#define MAX_OVERALL_COST_BOUND 600
-
 /* Major function to choose the current insn alternative and what
operands should be reloaded and how. If ONLY_ALTERNATIVE is not
negative we should consider only this alternative.  Return false if
@@ -1576,6 +1570,7 @@ process_alt_operands (int only_alternati
badop = false;
this_alternative = curr_alt[m];
COPY_HARD_REG_SET (this_alternative_set, curr_alt_set[m]);
+   winreg = this_alternative != NO_REGS;
break;
  }
 
@@ -1828,7 +1823,7 @@ process_alt_operands (int only_alternati
 might cost something but probably less than old
 reload pass believes.  */
  if (lra_former_scratch_p (REGNO (operand_reg[nop])))
-   reject += LOSER_COST_FACTOR;
+   reject += LRA_LOSER_COST_FACTOR;
}
}
  else if (did_match)
@@ -1912,20 +1907,15 @@ process_alt_operands (int only_alternati
  && no_input_reloads_p && ! const_to_mem))
goto fail;
 
- /* If we can't reload this value at all, reject this
-alternative.  Note that we could also lose due to
-LIMIT_RELOAD_CLASS, but we don't check that here.  */
- if (! CONSTANT_P (op) && ! no_regs_p)
-   {
- if (targetm.preferred_reload_class
- (op, this_alternative) == NO_REGS)
-   reject = MAX_OVERALL_COST_BOUND;
-
- if (curr_static_id->operand[nop].type == OP_OUT
- && (targetm.preferred_output_reload_class
- (op, this_alternative) == NO_REGS))
-   reject = MAX_OVERALL_COST_BOUND;
-   }
+ /* Check strong discouragement of reload of non-constant
+into class THIS_ALTERNATIVE.  */
+ if (! CONSTANT_P (op) && ! no_regs_p
+ && (targetm.preferred_reload_class
+ (op, this_alternative) == NO_REGS
+ || (curr_static_id->operand[nop].type == OP_OUT
+ && (targetm.preferred_output_reload_class
+ (op, this_alternative) == NO_REGS
+   reject += LRA_MAX_REJECT;
 
  if (!

Re: [PATCH] PR tree-optimization/55079: Don't remove all exits of a loop during loop unroll

[Ian Lance Taylor]

On Fri, Nov 9, 2012 at 5:11 AM, Siddhesh Poyarekar  wrote:
>
> I understand that the behaviour is undefined, but this is easily
> avoidable by skipping removal of the exits if it results in an infinite
> loop.  Attached patch does exactly that.

I don't mind saying that GCC should define cases that the language
standards leave undefined.  But that does not seem to be what this
patch does.  I don't see why this is a good idea.  It seems to produce
a program that is unpredictable in a different way.  How does that
help the user?  If anything an infinite loop might be better, since it
is more obvious that something is wrong.  Unless I misunderstand.

Ian

Re: [PATCH] Document __ATOMIC_HLE_ACQUIRE/RELEASE

[Jakub Jelinek]

On Fri, Nov 09, 2012 at 04:04:15PM +0100, Andi Kleen wrote:
> +Additional target specific flags that can be ored with the memory models.
> +@table  @code
> +@item __ATOMIC_HLE_ACQUIRE
> +(on i386/x86_64). Start a Hardware Lock Elision transaction with the atomic
> +operation. The memory model must be @code{__ATOMIC_ACQUIRE} or stronger.

Two spaces after . instead of just one.

> +@item __ATOMIC_HLE_RELEASE
> +(on i386/x86_64). Commit a Hardware Lock Elision transaction with the atomic
> +operation. The memory model must be @code{__ATOMIC_RELEASE} or stronger.
> +@end table
> +
>  When implementing patterns for these built-in functions, the memory model
>  parameter can be ignored as long as the pattern implements the most
>  restrictive @code{__ATOMIC_SEQ_CST} model.  Any of the other memory models

Jakub

Re: [PATCH] Handle target specific memory models in C frontend

[Jakub Jelinek]

Hi!

Just nits.

On Fri, Nov 09, 2012 at 04:03:47PM +0100, Andi Kleen wrote:
> From: Andi Kleen 
> 
> get_atomic_generic_size would error out for
> __atomic_store(...,__ATOMIC_HLE_RELEASE)
> 
> Just mask it out. All the memory orders are checked completely
> in builtins.c anyways.
> 
> I'm not sure what that check is for, it could be removed in theory.

I think it actually should be removed, given that e.g.
int i, j;
void
foo (void)
{
  __atomic_store (&i, &j, 12);
}
warns twice for the same thing, when it should only warn once.
Richard?

> Passed bootstrap and test suite on x86-64
> 
> gcc/c-family/:
> 2012-11-09  Andi Kleen  
> 
>   PR55139

That should be PR target/55139

>   * c-common.c (get_atomic_generic_size): Mask with
> MEMMODEL_MASK

Dot after MEMMODEL_MASK and don't wrap it on next line.

> ---
>  gcc/c-family/c-common.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/gcc/c-family/c-common.c b/gcc/c-family/c-common.c
> index c3a6465..7828d21 100644
> --- a/gcc/c-family/c-common.c
> +++ b/gcc/c-family/c-common.c
> @@ -10097,7 +10097,7 @@ get_atomic_generic_size (location_t loc, tree 
> function, VEC(tree,gc) *params)
>if (TREE_CODE (p) == INTEGER_CST)
>  {
> int i = tree_low_cst (p, 1);
> -   if (i < 0 || i >= MEMMODEL_LAST)
> +   if (i < 0 || (i & MEMMODEL_MASK) >= MEMMODEL_LAST)
>   {
> warning_at (loc, OPT_Winvalid_memory_model,
> "invalid memory model argument %d of %qE", x + 1,

If that whole loop doesn't get removed, there are some formatting issues in
it:
for (x = n_param - n_model ; x < n_param; x++)
shouldn't have space before ; and 
error_at (loc, "non-integer memory model argument %d of %qE", x + 1,
   function);
doesn't correctly horizontally aling the second line.

Jakub

Re: [PATCH] Fix fold reassociation (PR c++/55137)

[Sebastian Huber]

Hello Jakub,

with your patch and 4.8.0 20121109 the problem in PR55137 vanishes and I am 
able to run the RTEMS testsuite on PowerPC again.  Thanks.


--
Sebastian Huber, embedded brains GmbH

Address : Obere Lagerstr. 30, D-82178 Puchheim, Germany
Phone   : +49 89 18 90 80 79-6
Fax : +49 89 18 90 80 79-9
E-Mail  : sebastian.hu...@embedded-brains.de
PGP : Public key available on request.

Diese Nachricht ist keine geschäftliche Mitteilung im Sinne des EHUG.

Re: [PATCH] Handle target specific memory models in C frontend

[Richard Henderson]

On 2012-11-09 07:03, Andi Kleen wrote:
>   PR55139
>   * c-common.c (get_atomic_generic_size): Mask with
> MEMMODEL_MASK

Ok.


r~

[PATCH] Document __ATOMIC_HLE_ACQUIRE/RELEASE

[Andi Kleen]

From: Andi Kleen 

Make a first stab at the undocumented HLE_ACQUIRE/RELEASE
memory model modifiers in the documentation

gcc/:
2012-11-09 Andi Kleen  

* doc/extend.texi: Document __ATOMIC_HLE_ACQUIRE and RELEASE.
---
 gcc/doc/extend.texi | 10 ++
 1 file changed, 10 insertions(+)

diff --git a/gcc/doc/extend.texi b/gcc/doc/extend.texi
index 54fd548..248977d 100644
--- a/gcc/doc/extend.texi
+++ b/gcc/doc/extend.texi
@@ -7194,6 +7194,16 @@ Full barrier in both directions and synchronizes with 
acquire loads and
 release stores in all threads.
 @end table
 
+Additional target specific flags that can be ored with the memory models.
+@table  @code
+@item __ATOMIC_HLE_ACQUIRE
+(on i386/x86_64). Start a Hardware Lock Elision transaction with the atomic
+operation. The memory model must be @code{__ATOMIC_ACQUIRE} or stronger.
+@item __ATOMIC_HLE_RELEASE
+(on i386/x86_64). Commit a Hardware Lock Elision transaction with the atomic
+operation. The memory model must be @code{__ATOMIC_RELEASE} or stronger.
+@end table
+
 When implementing patterns for these built-in functions, the memory model
 parameter can be ignored as long as the pattern implements the most
 restrictive @code{__ATOMIC_SEQ_CST} model.  Any of the other memory models
-- 
1.7.11.5

[PATCH] Handle target specific memory models in C frontend

[Andi Kleen]

From: Andi Kleen 

get_atomic_generic_size would error out for
__atomic_store(...,__ATOMIC_HLE_RELEASE)

Just mask it out. All the memory orders are checked completely
in builtins.c anyways.

I'm not sure what that check is for, it could be removed in theory.

Passed bootstrap and test suite on x86-64

gcc/c-family/:
2012-11-09  Andi Kleen  

PR55139
* c-common.c (get_atomic_generic_size): Mask with
MEMMODEL_MASK
---
 gcc/c-family/c-common.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/gcc/c-family/c-common.c b/gcc/c-family/c-common.c
index c3a6465..7828d21 100644
--- a/gcc/c-family/c-common.c
+++ b/gcc/c-family/c-common.c
@@ -10097,7 +10097,7 @@ get_atomic_generic_size (location_t loc, tree function, 
VEC(tree,gc) *params)
   if (TREE_CODE (p) == INTEGER_CST)
 {
  int i = tree_low_cst (p, 1);
- if (i < 0 || i >= MEMMODEL_LAST)
+ if (i < 0 || (i & MEMMODEL_MASK) >= MEMMODEL_LAST)
{
  warning_at (loc, OPT_Winvalid_memory_model,
  "invalid memory model argument %d of %qE", x + 1,
-- 
1.7.11.5

Re: Asan/Tsan Unit/Regression testing (was [asan] Emit GIMPLE direclty, small cleanups)

[Jakub Jelinek]

On Wed, Nov 07, 2012 at 07:02:04AM -0800, Kostya Serebryany wrote:
> 1. LLVM unittests (Text file with LLVM IR and the expected result of
> the transformations).
> Example:  
> http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Instrumentation/AddressSanitizer/do-not-touch-threadlocal.ll?revision=145092&view=markup
> I am not sure if anything similar is possible with GCC.

We don't have a GIMPLE FE (yet), so we need to feed in C or C++ or Fortran
etc. as source (which isn't that hard though), but can scan gimple and/or
rtl dumps and/or assembly for required transformations easily.

> 2. Large Gtest-based unittest. This is a set of c++ files that should
> be built with the asan switch, depends on gtest
> (http://code.google.com/p/googletest/).
> http://llvm.org/viewvc/llvm-project/compiler-rt/trunk/lib/asan/tests/asan_test.cc?revision=166104&view=markup
> This should be easy to port to GCC, but it requires gtest.

I don't think we want to depend on gtest, if the tests only use a small
subset of that, it would be better to just use some header with macros
compatible with that for assertions and the like.

> 3. Full output tests (a .cc file should be build with asan switch,
> executable should be run and the stderr is compared with the expected
> output)
> Example: 
> http://llvm.org/viewvc/llvm-project/compiler-rt/trunk/lib/asan/lit_tests/stack-overflow.cc?revision=165391&view=markup
> The can be ported to GCC, but the uses of FileCheck
> (http://llvm.org/docs/CommandGuide/FileCheck.html) will need to be
> replaced with GCC's analog.
> We should probably start with these tests.

Dejagnu in GCC has

! { dg-do run }
! { dg-options "-fbounds-check" }
! { dg-shouldfail "Duplicate value 2 in ORDER argument to RESHAPE intrinsic" }
program main
  implicit none
  integer(kind=1), dimension(6) :: source1 = (/ 1, 2, 3, 4, 5, 6 /)
  integer, dimension(2) :: shape1 = (/ 2, 3/)
  integer(kind=1), dimension(2) :: pad1 = (/ 0, 0/)
  character(len=200) :: l1, l2
  integer :: i1, i2

  l1 = "2 2"
  read(unit=l1,fmt=*) i1, i2
  write (unit=l2,fmt=*) reshape(source1, shape1, pad1, (/i1, i2/)) ! Invalid
end program main
! { dg-output "Fortran runtime error: Duplicate value 2 in ORDER argument to 
RESHAPE intrinsic" }

style markings, dg-shouldfail says that the program is expected to fail
rather than pass (if it aborts), and dg-output (perhaps multiple) can
contain regexps to match against stderr + stdout joined.  Haven't looked
at the asan tests yet, do you expect just one ASAN abort per test, or do you
say somehow catch the failure exception, longjmp to some safe point and
continue with another crash?  I mean something similar to glibc
-D_FORTIFY_SOURCE tests,
http://sourceware.org/git/?p=glibc.git;a=blob;f=debug/tst-chk1.c;h=7ffc9fec9084a1f536c562d4dc3861055b7b5788;hb=HEAD
?

Jakub

Re: patch fixing a test for PR55151

[Vladimir Makarov]

On 12-11-08 11:59 AM, Hans-Peter Nilsson wrote:

On Wed, 7 Nov 2012, Vladimir Makarov wrote:

On 12-11-07 5:27 PM, H.J. Lu wrote:

You should check !ia32 target:

/* { dg-do compile { target { ! { ia32 } } } } */



Thanks, H.J.  I've just fixed it.

Index: testsuite/ChangeLog
===
--- testsuite/ChangeLog (revision 193316)
+++ testsuite/ChangeLog (working copy)
@@ -1,3 +1,8 @@
+2012-11-07  Vladimir Makarov  
+
+   PR rtl-optimization/55151
+   * gcc.dg/pr55151.c: Use ia32 instead of x86_64.
+

Lots of constraints there that might not be suitable for all
machines.  Unless it's expected to pass (almost) everywhere, it
should move to gcc.target/i386.

I am agree it is better not to run it on any target.  I've moved it to 
gcc.target/i386.


Thanks for pointing this out.

Re: [PATCH 01/10] Initial import of asan from the Google branch into trunk

[Jakub Jelinek]

On Fri, Nov 09, 2012 at 02:14:04PM +0100, Tobias Burnus wrote:
> Dodji Seketeli wrote:
> >This patch imports the initial state of asan as it was in the
> >Google branch.
> >
> >It provides basic infrastructure for asan to instrument memory
> >accesses on the heap, at -O3.  Note that it supports neither stack nor
> >global variable protection.
> 
> I tried the 01/10 to 10/10 patch series but it doesn't trigger for
> the following test case:
> 
> #include 
> #include 
> 
> int
> main() {
>   int *i;
>   i = malloc(10*sizeof(*i));
>   free(i);  /* <<< Free memory. */
>   i[10] = 5;  /* <<< out of boundary even if not freed. */
>   printf("%d\n", i[11]);  /* <<< out of boundary even if not freed. */
>   return 0;
> }
> 
> (All of them are reported by Clang.) If I look at the dump (or
> assembler), I see the call to __asan_init, __asan_report_store4 and
> __asan_report_load4. However, when running the program ltrace only
> shows the calls to: __libc_start_main, __asan_init, malloc, free and
> printf. I haven't debugged why the condition is false [see
> attachment for the dump].

Can't reproduce that (admittedly with asan SVN branch rather than the
patchset):

./xgcc -B ./ -O2 -fasan -o a a.c 
-Wl,-rpath,/usr/src/gcc-asan/obj/x86_64-unknown-linux-gnu/libsanitizer/asan/.libs/
 \
 -L 
/usr/src/gcc-asan/obj/x86_64-unknown-linux-gnu/libsanitizer/asan/.libs/
./a
=
==20614== ERROR: AddressSanitizer heap-use-after-free on address
0x7f7d8245afec at pc 0x4006f8 bp 0x7fff9beda4c0 sp 0x7fff9beda4b8
READ of size 4 at 0x7f7d8245afec thread T0
#0 0x4006f7 (/usr/src/gcc-asan/obj/gcc/a+0x4006f7)
0x7f7d8245afec is located 4 bytes to the right of 40-byte region
[0x7f7d8245afc0,0x7f7d8245afe8)
freed by thread T0 here:
#0 0x7f7d82796585

#(/usr/src/gcc-asan/obj/x86_64-unknown-linux-gnu/libsanitizer/asan/.libs/libasan.so.0.0.0+0xf585)
#1 0x4006b5 (/usr/src/gcc-asan/obj/gcc/a+0x4006b5)
previously allocated by thread T0 here:
#0 0x7f7d82796645

#(/usr/src/gcc-asan/obj/x86_64-unknown-linux-gnu/libsanitizer/asan/.libs/libasan.so.0.0.0+0xf645)
#1 0x4006aa (/usr/src/gcc-asan/obj/gcc/a+0x4006aa)
Shadow byte and word:
  0x1fefb048b5fd: fd
  0x1fefb048b5f8: fd fd fd fd fd fd fd fd
More shadow bytes:
  0x1fefb048b5d8: fa fa fa fa fa fa fa fa
  0x1fefb048b5e0: fa fa fa fa fa fa fa fa
  0x1fefb048b5e8: fa fa fa fa fa fa fa fa
  0x1fefb048b5f0: fa fa fa fa fa fa fa fa
=>0x1fefb048b5f8: fd fd fd fd fd fd fd fd
  0x1fefb048b600: fa fa fa fa fa fa fa fa
  0x1fefb048b608: fa fa fa fa fa fa fa fa
  0x1fefb048b610: fa fa fa fa fa fa fa fa
  0x1fefb048b618: fa fa fa fa fa fa fa fa
Stats: 0M malloced (0M for red zones) by 1 calls
Stats: 0M realloced by 0 calls
Stats: 0M freed by 1 calls
Stats: 0M really freed by 0 calls
Stats: 0M (128 full pages) mmaped in 1 calls
  mmaps   by size class: 7:4095; 
  mallocs by size class: 7:1; 
  frees   by size class: 7:1; 
  rfrees  by size class: 
Stats: malloc large: 0 small slow: 1
==20614== ABORTING

Jakub

Re: [PATCH Version 2][RFA]Improving register pressure directed hoist

[Jeff Law]

On 11/07/2012 02:05 PM, Bin Cheng wrote:


2012-11-08  Bin Cheng  

* gcse.c: (struct bb_data): Add new fields, old_pressure, live_in
and backup.
(calculate_bb_reg_pressure): Initialize live_in and backup.
(update_bb_reg_pressure): New.
(should_hoist_expr_to_dom): Add new parameter from.
Monitor the change of reg pressure and use it to drive hoisting.
(hoist_code): Update LIVE and reg pressure information.

gcc/testsuite/ChangeLog
2012-11-08  Bin Cheng  

* gcc.dg/hoist-register-pressure-3.c: New test.

Thanks for taking care of that inconsistency.  This is OK to install now.

Thanks,
jeff

Re: [Darwin] Do not enable -fvar-tracking at -O0

[Jack Howarth]

On Fri, Nov 09, 2012 at 10:42:45AM +0100, Eric Botcazou wrote:
> Hi,
> 
> this is a regression present on the mainline and 4.7 branch.  -fvar-tracking 
> is automatically enabled on Darwin when -g is passed, including at -O0.  This 
> results in bloated and incomplete debug info.
> 
> This comes from a latent issue in the handling of flag_var_tracking_uninit in 
> the Darwin back-end, which wasn't updated for the new option machinery.

Eric,
   Is this a hack around the underlying problem on darwin or the implementation
of the same approach used on other targets? If the former, shouldn't we have a
PR opened to properly fix this in the long term?
 Jack

> 
> Tested on x86_64-apple-darwin10.8.0, OK for mainline and 4.7 branch?
> 
> 
> 2012-11-09  Eric Botcazou  
> 
>   * common.opt (fvar-tracking-uninit): Document.
>   * toplev.c (process_options): Fix handling of flag_var_tracking_uninit.
>   * config/darwin.c (darwin_override_options): Likewise.
> 
> 
> -- 
> Eric Botcazou
> Index: toplev.c
> ===
> --- toplev.c  (revision 193322)
> +++ toplev.c  (working copy)
> @@ -1416,12 +1416,15 @@ process_options (void)
>/* If the user specifically requested variable tracking with tagging
>   uninitialized variables, we need to turn on variable tracking.
>   (We already determined above that variable tracking is feasible.)  */
> -  if (flag_var_tracking_uninit)
> +  if (flag_var_tracking_uninit == 1)
>  flag_var_tracking = 1;
>  
>if (flag_var_tracking == AUTODETECT_VALUE)
>  flag_var_tracking = optimize >= 1;
>  
> +  if (flag_var_tracking_uninit == AUTODETECT_VALUE)
> +flag_var_tracking_uninit = flag_var_tracking;
> +
>if (flag_var_tracking_assignments == AUTODETECT_VALUE)
>  flag_var_tracking_assignments = flag_var_tracking
>&& !(flag_selective_scheduling || flag_selective_scheduling2);
> Index: common.opt
> ===
> --- common.opt(revision 193322)
> +++ common.opt(working copy)
> @@ -2189,6 +2189,10 @@ fvar-tracking-assignments-toggle
>  Common Report Var(flag_var_tracking_assignments_toggle) Optimization
>  Toggle -fvar-tracking-assignments
>  
> +; Positive if we should track uninitialized variables, negative if
> +; we should run the var-tracking pass only to discard debug
> +; annotations.  When flag_var_tracking_uninit == AUTODETECT_VALUE it
> +; will be set according to flag_var_tracking.
>  fvar-tracking-uninit
>  Common Report Var(flag_var_tracking_uninit) Optimization
>  Perform variable tracking and also tag variables that are uninitialized
> Index: config/darwin.c
> ===
> --- config/darwin.c   (revision 193322)
> +++ config/darwin.c   (working copy)
> @@ -3038,12 +3038,12 @@ darwin_override_options (void)
>darwin_emit_branch_islands = true;
>  }
>  
> -  if (flag_var_tracking
> +  if (flag_var_tracking_uninit == 0
>&& generating_for_darwin_version >= 9
>&& (flag_gtoggle ? (debug_info_level == DINFO_LEVEL_NONE)
>: (debug_info_level >= DINFO_LEVEL_NORMAL))
>&& write_symbols == DWARF2_DEBUG)
> -flag_var_tracking_uninit = 1;
> +flag_var_tracking_uninit = flag_var_tracking;
>  
>if (MACHO_DYNAMIC_NO_PIC_P)
>  {

Re: [SH] Enable shrink-wrap with reorder_blocks_and_parition

[Kaz Kojima]

Christian Bruel  wrote:
> Now that the cross-jumping problem is fixed since rev #193350, I'd like
> to remove this restriction and close PR/54546.
> 
> Checked with default sh-sim target_board and
> --target_board=sh-sim/-freorder-blocks-and-partition.

OK.

Regards,
kaz

Re: [off-list] Re: [PATCH] Vzeroupper placement/47440

[Uros Bizjak]

On Fri, Nov 9, 2012 at 1:47 PM, Uros Bizjak  wrote:

>>> >> These assert should tell you what is wrong with the control flow.
>>> >> Please look at control_flow_insn_p, which condition returns true.
>>> >
>>> > There is a note after call insn.
>>> >
>>> > (call_insn:TI 908 35558 50534 1681 (call (mem:QI (symbol_ref:DI
>>> > ("_gfortran_stop_string") [flags 0x41] >> > _gfortran_stop_string>) [0 _gfortran_stop_string S1 A8])
>>> > (const_int 0 [0])) huygens.fppized.f90:190 616 {*call}
>>> >  (expr_list:REG_DEAD (reg:DI 5 di)
>>> > (expr_list:REG_DEAD (reg:SI 4 si)
>>> > (expr_list:REG_NORETURN (const_int 0 [0])
>>> > (nil
>>> > (expr_list:REG_FRAME_RELATED_EXPR (use (reg:DI 5 di))
>>> > (expr_list:REG_BR_PRED (use (reg:SI 4 si))
>>> > (nil
>>> > (note 50534 908 909 1681 (expr_list:REG_DEP_TRUE (concat:DI (reg:DI 5 di)
>>> > (const_int 0 [0]))
>>> > (expr_list:REG_DEP_TRUE (concat:SI (reg:SI 4 si)
>>> > (const_int 0 [0]))
>>> > (nil))) NOTE_INSN_CALL_ARG_LOCATION)
>>> >
>>>
>>> Huh, this RTX is ignored:
>>
>> NOTE_INSN_CALL_ARG_LOCATION is fine, even after a REG_NORETURN call.
>> It is just a way how to pass call argument details to dwarf2out.
>> If you have a pass after var-tracking, you need to skip over it.
>
> Yes, but postreload mode switching should come before pro_and_epilogue
> anyway, otherwise create_pre_exit won't work:
>
> --mode-switching.c (222)--
> /* If this function returns a value at the end, we have to
>insert the final mode switch before the return value copy
>to its hard register.  */
> if (EDGE_COUNT (EXIT_BLOCK_PTR->preds) == 1
> && NONJUMP_INSN_P ((last_insn = BB_END (src_bb)))
> && GET_CODE (PATTERN (last_insn)) == USE
> && GET_CODE ((ret_reg = XEXP (PATTERN (last_insn), 0))) == REG)
> --mode-switching.2 (228)--
>
> I believe that this will work OK if the pass is inserted before
> prologue generation pass.

Finally, having a post-reload mode-switching pass, we can double-check
that there are no live SSE registers at vzeroupper insertion point. As
vzeroupper is only an optimization, we want to play safe and cancel
vzeroupper insertion in this case

There is no degradation for x86_64 gABI targets, since all SSE
registers are call-clobbered. Vzeroupper is conditionally inserted
just before call insn, where all registers are saved to stack and
already dead. The vzeroupper at function exit is not problematic.

Uros.
Index: i386-protos.h
===
--- i386-protos.h   (revision 193329)
+++ i386-protos.h   (working copy)
@@ -172,8 +172,11 @@ extern int ix86_mode_needed (int, rtx);
 extern int ix86_mode_after (int, int, rtx);
 extern int ix86_mode_entry (int);
 extern int ix86_mode_exit (int);
-extern void ix86_emit_mode_set (int, int);
 
+#ifdef HARD_CONST
+extern void ix86_emit_mode_set (int, int, HARD_REG_SET);
+#endif
+
 extern void x86_order_regs_for_local_alloc (void);
 extern void x86_function_profiler (FILE *, int);
 extern void x86_emit_floatuns (rtx [2]);
Index: i386.c
===
--- i386.c  (revision 193329)
+++ i386.c  (working copy)
@@ -15277,16 +15284,38 @@ emit_i387_cw_initialization (int mode)
   emit_move_insn (new_mode, reg);
 }
 
+/* Emit vzeroupper.  */
+
+void
+ix86_avx_emit_vzeroupper (HARD_REG_SET regs_live)
+{
+  int i;
+
+  /* Cancel automatic vzeroupper insertion if there are
+ live call-saved SSE registers at the insertion point.  */
+
+  for (i = FIRST_SSE_REG; i <= LAST_SSE_REG; i++)
+if (!call_used_regs[i] && TEST_HARD_REG_BIT (regs_live, i))
+  return;
+
+  if (TARGET_64BIT)
+for (i = FIRST_REX_SSE_REG; i <= LAST_REX_SSE_REG; i++)
+  if (!call_used_regs[i] && TEST_HARD_REG_BIT (regs_live, i))
+   return;
+
+  emit_insn (gen_avx_vzeroupper ());
+}
+
 /* Generate one or more insns to set ENTITY to MODE.  */
 
 void
-ix86_emit_mode_set (int entity, int mode)
+ix86_emit_mode_set (int entity, int mode, HARD_REG_SET regs_live)
 {
   switch (entity)
 {
 case AVX_U128:
   if (mode == AVX_U128_CLEAN)
-   emit_insn (gen_avx_vzeroupper ());
+   ix86_avx_emit_vzeroupper (regs_live);
   break;
 case I387_TRUNC:
 case I387_FLOOR:
Index: i386.h
===
--- i386.h  (revision 193329)
+++ i386.h  (working copy)
@@ -2223,7 +2227,7 @@ enum avx_u128_state
are to be inserted.  */
 
 #define EMIT_MODE_SET(ENTITY, MODE, HARD_REGS_LIVE) \
-  ix86_emit_mode_set ((ENTITY), (MODE))
+  ix86_emit_mode_set ((ENTITY), (MODE), (HARD_REGS_LIVE))
 
 /* Avoid renaming of stack registers, as doing so in combination with
scheduling just increases amount of live registers at time and in

Re: patch to fix constant math - third small patch

[Kenneth Zadeck]

This patch is an updated version of my patch 3 with all of richard 
sandiford's comments resolved.


Richi had approved it before he went on vacation.

Committed as revision 193360.

Kenny


On 10/08/2012 03:42 PM, Richard Sandiford wrote:

Kenneth Zadeck  writes:

diff --git a/gcc/combine.c b/gcc/combine.c
index 4e0a579..b531305 100644
--- a/gcc/combine.c
+++ b/gcc/combine.c
@@ -2617,16 +2617,19 @@ try_combine (rtx i3, rtx i2, rtx i1, rtx i0, int 
*new_direct_jump_p,
   constant.  */
if (i1 == 0
&& (temp = single_set (i2)) != 0
-  && (CONST_INT_P (SET_SRC (temp))
- || CONST_DOUBLE_AS_INT_P (SET_SRC (temp)))
+  && CONST_SCALAR_INT_P (SET_SRC (temp))
&& GET_CODE (PATTERN (i3)) == SET
-  && (CONST_INT_P (SET_SRC (PATTERN (i3)))
- || CONST_DOUBLE_AS_INT_P (SET_SRC (PATTERN (i3
+  && CONST_SCALAR_INT_P (SET_SRC (PATTERN (i3)))
&& reg_subword_p (SET_DEST (PATTERN (i3)), SET_DEST (temp)))
  {
rtx dest = SET_DEST (PATTERN (i3));
int offset = -1;
int width = 0;
+
+  /* There are not explicit tests to make sure that this is not a
+float, but there is code here that would not be correct if it
+was.  */
+  gcc_assert (GET_MODE_CLASS (GET_MODE (SET_SRC (temp))) != MODE_FLOAT);

No need for this assert: CONST_SCALAR_INT_P (SET_SRC (temp)) should cover it.


@@ -1009,9 +1007,7 @@ rtx_equal_for_cselib_1 (rtx x, rtx y, enum machine_mode 
memmode)
  static rtx
  wrap_constant (enum machine_mode mode, rtx x)
  {
-  if (!CONST_INT_P (x)
-  && GET_CODE (x) != CONST_FIXED
-  && !CONST_DOUBLE_AS_INT_P (x))
+  if ((!CONST_SCALAR_INT_P (x)) && GET_CODE (x) != CONST_FIXED)

Redundant brackets.

Looks good to me otherwise, thanks.

Richard


2012-11-09  Kenneth Zadeck 

* rtl.h (CONST_SCALAR_INT_P): New macro.
* cfgexpand.c (expand_debug_locations): Changed to use
CONST_SCALAR_INT_P macro.
* combine.c (try_combine, subst, make_extraction,
gen_lowpart_for_combine): Ditto.
* cselib.c (entry_and_rtx_equal_p, rtx_equal_for_cselib_1): Ditto.
* dwarf2out.c (loc_descriptor): Ditto.
* emit-rtl.c (gen_lowpart_common): Ditto.
* ira-costs.c (record_reg_classes, record_address_regs): Ditto.
* ira-lives.c (single_reg_class): Ditto.
* recog.c (simplify_while_replacing, asm_operand_ok,
constrain_operands): Ditto.
* reload.c (find_reloads): Ditto.
* simplify-rtx.c (simplify_unary_operation_1,
simplify_const_unary_operation, simplify_binary_operation_1,
simplify_const_binary_operation, simplify_relational_operation_1,
simplify_subreg): Ditto.

diff --git a/gcc/cfgexpand.c b/gcc/cfgexpand.c
index e501b4b..0bd9d1d 100644
--- a/gcc/cfgexpand.c
+++ b/gcc/cfgexpand.c
@@ -3647,9 +3647,8 @@ expand_debug_locations (void)
 
 	gcc_assert (mode == GET_MODE (val)
 			|| (GET_MODE (val) == VOIDmode
-			&& (CONST_INT_P (val)
+			&& (CONST_SCALAR_INT_P (val)
 || GET_CODE (val) == CONST_FIXED
-|| CONST_DOUBLE_AS_INT_P (val) 
 || GET_CODE (val) == LABEL_REF)));
 	  }
 
diff --git a/gcc/combine.c b/gcc/combine.c
index 00719a7..5e85f77 100644
--- a/gcc/combine.c
+++ b/gcc/combine.c
@@ -2619,17 +2619,15 @@ try_combine (rtx i3, rtx i2, rtx i1, rtx i0, int *new_direct_jump_p,
  constant.  */
   if (i1 == 0
   && (temp = single_set (i2)) != 0
-  && (CONST_INT_P (SET_SRC (temp))
-	  || CONST_DOUBLE_AS_INT_P (SET_SRC (temp)))
+  && CONST_SCALAR_INT_P (SET_SRC (temp))
   && GET_CODE (PATTERN (i3)) == SET
-  && (CONST_INT_P (SET_SRC (PATTERN (i3)))
-	  || CONST_DOUBLE_AS_INT_P (SET_SRC (PATTERN (i3
+  && CONST_SCALAR_INT_P (SET_SRC (PATTERN (i3)))
   && reg_subword_p (SET_DEST (PATTERN (i3)), SET_DEST (temp)))
 {
   rtx dest = SET_DEST (PATTERN (i3));
   int offset = -1;
   int width = 0;
-
+  
   if (GET_CODE (dest) == ZERO_EXTRACT)
 	{
 	  if (CONST_INT_P (XEXP (dest, 1))
@@ -5104,8 +5102,7 @@ subst (rtx x, rtx from, rtx to, int in_dest, int in_cond, int unique_copy)
 	  if (GET_CODE (new_rtx) == CLOBBER && XEXP (new_rtx, 0) == const0_rtx)
 		return new_rtx;
 
-	  if (GET_CODE (x) == SUBREG
-		  && (CONST_INT_P (new_rtx) || CONST_DOUBLE_AS_INT_P (new_rtx)))
+	  if (GET_CODE (x) == SUBREG && CONST_SCALAR_INT_P (new_rtx))
 		{
 		  enum machine_mode mode = GET_MODE (x);
 
@@ -7134,7 +7131,7 @@ make_extraction (enum machine_mode mode, rtx inner, HOST_WIDE_INT pos,
   if (mode == tmode)
 	return new_rtx;
 
-  if (CONST_INT_P (new_rtx) || CONST_DOUBLE_AS_INT_P (new_rtx))
+  if (CONST_SCALAR_INT_P (new_rtx))
 	return simplify_unary_operation (unsignedp ? ZERO_EXTEND : SIGN_EXTEND,
 	 mode, new_rtx, tmode);
 
@@ -10658,8 +10655,7 @@ gen_lowpart_for_combine (enum machine_mode omode, rtx x)
   /* We can only support MODE being wider than a word if X is a
  constant integer or has a mode the same size.  */
   

Re: [PATCH 01/10] Initial import of asan from the Google branch into trunk

[Tobias Burnus]

Dodji Seketeli wrote:

This patch imports the initial state of asan as it was in the
Google branch.

It provides basic infrastructure for asan to instrument memory
accesses on the heap, at -O3.  Note that it supports neither stack nor
global variable protection.


I tried the 01/10 to 10/10 patch series but it doesn't trigger for the 
following test case:


#include 
#include 

int
main() {
  int *i;
  i = malloc(10*sizeof(*i));
  free(i);  /* <<< Free memory. */
  i[10] = 5;  /* <<< out of boundary even if not freed. */
  printf("%d\n", i[11]);  /* <<< out of boundary even if not freed. */
  return 0;
}

(All of them are reported by Clang.) If I look at the dump (or 
assembler), I see the call to __asan_init, __asan_report_store4 and 
__asan_report_load4. However, when running the program ltrace only shows 
the calls to: __libc_start_main, __asan_init, malloc, free and printf. I 
haven't debugged why the condition is false [see attachment for the dump].



Other issues:

* libasan does not seem to be a multilib, at least I only find the 64bit 
version on x86-64-gnu-linux such that "-m32" compilation fails.


* -fno-address-sanitizer doesn't work (it does in Clang); it is 
explicitly disabled via RejectNegative in gcc/common.opt


* Probably fixed on the branch: gcc/gcc.c still has "fasan" instead of 
"faddress-sanitizer" for the spec:

+%{fasan:-lasan}

Tobias
#include 
#include 

int
main() {
  int *i;
  i = malloc(10*sizeof(*i));
  free(i);  /* <<< Free memory. */
  i[10] = 5;  /* <<< out of boundary even if not freed. */
  printf("%d\n", i[11]);  /* <<< out of boundary even if not freed. */
  return 0;
}

;; Function main (main, funcdef_no=2, decl_uid=2680, cgraph_uid=2)

main ()
{
  int * i;
  int D.2687;
  int D.2686;
  int * D.2685;
  int * D.2684;
  int * _2;
  int * _3;
  int _4;
  int _5;
  unsigned long _6;
  unsigned long _7;
  unsigned long _8;
  unsigned char * _9;
  unsigned char _10;
  _Bool _11;
  unsigned long _12;
  unsigned char _13;
  unsigned char _14;
  _Bool _15;
  _Bool _16;
  unsigned long _17;
  unsigned long _18;
  unsigned long _19;
  unsigned char * _20;
  unsigned char _21;
  _Bool _22;
  unsigned long _23;
  unsigned char _24;
  unsigned char _25;
  _Bool _26;
  _Bool _27;

  :
  i_1 = malloc (40);
  free (i_1);
  _2 = i_1 + 40;
  _6 = (unsigned long) _2;
  _7 = _6 >> 3;
  _8 = _7 + 17592186044416;
  _9 = (unsigned char *) _8;
  _10 = *_9;
  _11 = _10 != 0;
  _12 = _6 & 7;
  _13 = (unsigned char) _12;
  _14 = _13 + 3;
  _15 = _14 >= _10;
  _16 = _11 & _15;
  if (_16 != 0)
goto ;
  else
goto ;

  :
  __asan_report_store4 (_6);

  :
  *_2 = 5;
  _3 = i_1 + 44;
  _17 = (unsigned long) _3;
  _18 = _17 >> 3;
  _19 = _18 + 17592186044416;
  _20 = (unsigned char *) _19;
  _21 = *_20;
  _22 = _21 != 0;
  _23 = _17 & 7;
  _24 = (unsigned char) _23;
  _25 = _24 + 3;
  _26 = _25 >= _21;
  _27 = _22 & _26;
  if (_27 != 0)
goto ;
  else
goto ;

  :
  __asan_report_load4 (_17);

  :
  _4 = *_3;
  printf ("%d\n", _4);
  _5 = 0;

:
  return _5;

}



;; Function _GLOBAL__sub_I_00099_0_main (_GLOBAL__sub_I_00099_0_main, 
funcdef_no=3, decl_uid=2700, cgraph_uid=0)

_GLOBAL__sub_I_00099_0_main ()
{
  :
  __asan_init ();
  return;

}

[PATCH] PR tree-optimization/55079: Don't remove all exits of a loop during loop unroll

[Siddhesh Poyarekar]

Hi,

r193098 introduced a change to the loop unroll behaviour, where exits
beyond nb_iterations_upper_bound were removed as being redundant.  This
assumption is not true for an undefined behaviour, which is when a loop
causes access beyond bounds of an array.  In such a case, all exits are
removed and the result is an infinite loop.  Essentially, the following
program results in an infinite loop with -O:

int d[16];

int
main (void)
{
  int k, satd = 0, dd;

  for (dd=d[k=0]; k<16;)
{
  satd += (dd < 0 ? -dd : dd);
  ++k;
  dd=d[k];
}

  return satd;
}

I understand that the behaviour is undefined, but this is easily
avoidable by skipping removal of the exits if it results in an infinite
loop.  Attached patch does exactly that.

I guess a further improvement to this (if the patch approach is valid)
would be to issue a warning to the user based on this analysis.  I am
trying to figure out a way to call the remove_redundant_iv_tests
function safely in tree-vrp so that even -O2 does not produce an
infinite loop for the above program (it has since r186592) and prints
the warning instead.  Simply calling it after max_loop_iterations
causes a regression in the testsuite that I haven't figured out yet.

I have tested the patch on x86_64 for C language and the testsuite
reports no regressions.  In fact, it seems to have fixed a previous
failure in gcc.dg/vect/slp-perm-9.c.

Regards,
Siddhesh

ChangeLog:

2012-11-09  Siddhesh Poyarekar  

PR tree-optimization/55079
* gcc/tree-ssa-loop-ivcanon.c (remove_redundant_iv_tests):
Avoid removing all exits of the loop.
diff --git a/gcc/tree-ssa-loop-ivcanon.c b/gcc/tree-ssa-loop-ivcanon.c
index 601223b..04bcd86 100644
--- a/gcc/tree-ssa-loop-ivcanon.c
+++ b/gcc/tree-ssa-loop-ivcanon.c
@@ -525,10 +525,22 @@ static bool
 remove_redundant_iv_tests (struct loop *loop)
 {
   struct nb_iter_bound *elt;
-  bool changed = false;
+  loop_exit *exit;
+  VEC(gimple, stack) *exit_stmts = VEC_alloc (gimple, stack, 16);
+  VEC(edge, stack) *exit_edges = VEC_alloc (edge, stack, 16);
+  int exits_left = 0, num_exits = 0;
 
   if (!loop->any_upper_bound)
-return false;
+goto out;
+
+  /* Count our exits.  */
+  for (exit = loop->exits->next; exit->e; exit = exit->next)
+num_exits++;
+
+  if (num_exits == 0)
+goto out;
+
+  exits_left = num_exits;
   for (elt = loop->bounds; elt; elt = elt->next)
 {
   /* Exit is pointless if it won't be taken before loop reaches
@@ -555,21 +567,40 @@ remove_redundant_iv_tests (struct loop *loop)
 	  || !loop->nb_iterations_upper_bound.ult
 		   (tree_to_double_int (niter.niter)))
 	continue;
-	  
+
+	  exits_left--;
+
+	  VEC_safe_push (gimple, stack, exit_stmts, elt->stmt);
+	  VEC_safe_push (edge, stack, exit_edges, exit_edge);
+	}
+}
+
+  /* Don't remove the exits if it leaves us with an infinite loop.  */
+  if (exits_left > 0)
+{
+  while (VEC_length (gimple, exit_stmts))
+{
+	  gimple stmt = VEC_pop (gimple, exit_stmts);
+	  edge exit_edge = VEC_pop (edge, exit_edges);
+
 	  if (dump_file && (dump_flags & TDF_DETAILS))
 	{
 	  fprintf (dump_file, "Removed pointless exit: ");
-	  print_gimple_stmt (dump_file, elt->stmt, 0, 0);
+	  print_gimple_stmt (dump_file, stmt, 0, 0);
 	}
 	  if (exit_edge->flags & EDGE_TRUE_VALUE)
-	gimple_cond_make_false (elt->stmt);
+	gimple_cond_make_false (stmt);
 	  else
-	gimple_cond_make_true (elt->stmt);
-	  update_stmt (elt->stmt);
-	  changed = true;
+	gimple_cond_make_true (stmt);
+	  update_stmt (stmt);
 	}
 }
-  return changed;
+
+out:
+  VEC_free (gimple, stack, exit_stmts);
+  VEC_free (edge, stack, exit_edges);
+
+  return (exits_left && exits_left < num_exits);
 }
 
 /* Stores loops that will be unlooped after we process whole loop tree. */

Re: [off-list] Re: [PATCH] Vzeroupper placement/47440

[Uros Bizjak]

On Fri, Nov 9, 2012 at 1:36 PM, Jakub Jelinek  wrote:
> On Fri, Nov 09, 2012 at 01:29:18PM +0100, Uros Bizjak wrote:
>> On Fri, Nov 9, 2012 at 1:18 PM, Vladimir Yakovlev  
>> wrote:
>> >> These assert should tell you what is wrong with the control flow.
>> >> Please look at control_flow_insn_p, which condition returns true.
>> >
>> > There is a note after call insn.
>> >
>> > (call_insn:TI 908 35558 50534 1681 (call (mem:QI (symbol_ref:DI
>> > ("_gfortran_stop_string") [flags 0x41] > > _gfortran_stop_string>) [0 _gfortran_stop_string S1 A8])
>> > (const_int 0 [0])) huygens.fppized.f90:190 616 {*call}
>> >  (expr_list:REG_DEAD (reg:DI 5 di)
>> > (expr_list:REG_DEAD (reg:SI 4 si)
>> > (expr_list:REG_NORETURN (const_int 0 [0])
>> > (nil
>> > (expr_list:REG_FRAME_RELATED_EXPR (use (reg:DI 5 di))
>> > (expr_list:REG_BR_PRED (use (reg:SI 4 si))
>> > (nil
>> > (note 50534 908 909 1681 (expr_list:REG_DEP_TRUE (concat:DI (reg:DI 5 di)
>> > (const_int 0 [0]))
>> > (expr_list:REG_DEP_TRUE (concat:SI (reg:SI 4 si)
>> > (const_int 0 [0]))
>> > (nil))) NOTE_INSN_CALL_ARG_LOCATION)
>> >
>>
>> Huh, this RTX is ignored:
>
> NOTE_INSN_CALL_ARG_LOCATION is fine, even after a REG_NORETURN call.
> It is just a way how to pass call argument details to dwarf2out.
> If you have a pass after var-tracking, you need to skip over it.

Yes, but postreload mode switching should come before pro_and_epilogue
anyway, otherwise create_pre_exit won't work:

--mode-switching.c (222)--
/* If this function returns a value at the end, we have to
   insert the final mode switch before the return value copy
   to its hard register.  */
if (EDGE_COUNT (EXIT_BLOCK_PTR->preds) == 1
&& NONJUMP_INSN_P ((last_insn = BB_END (src_bb)))
&& GET_CODE (PATTERN (last_insn)) == USE
&& GET_CODE ((ret_reg = XEXP (PATTERN (last_insn), 0))) == REG)
--mode-switching.2 (228)--

I believe that this will work OK if the pass is inserted before
prologue generation pass.

Uros.

Re: [off-list] Re: [PATCH] Vzeroupper placement/47440

[Jakub Jelinek]

On Fri, Nov 09, 2012 at 01:29:18PM +0100, Uros Bizjak wrote:
> On Fri, Nov 9, 2012 at 1:18 PM, Vladimir Yakovlev  
> wrote:
> >> These assert should tell you what is wrong with the control flow.
> >> Please look at control_flow_insn_p, which condition returns true.
> >
> > There is a note after call insn.
> >
> > (call_insn:TI 908 35558 50534 1681 (call (mem:QI (symbol_ref:DI
> > ("_gfortran_stop_string") [flags 0x41]  > _gfortran_stop_string>) [0 _gfortran_stop_string S1 A8])
> > (const_int 0 [0])) huygens.fppized.f90:190 616 {*call}
> >  (expr_list:REG_DEAD (reg:DI 5 di)
> > (expr_list:REG_DEAD (reg:SI 4 si)
> > (expr_list:REG_NORETURN (const_int 0 [0])
> > (nil
> > (expr_list:REG_FRAME_RELATED_EXPR (use (reg:DI 5 di))
> > (expr_list:REG_BR_PRED (use (reg:SI 4 si))
> > (nil
> > (note 50534 908 909 1681 (expr_list:REG_DEP_TRUE (concat:DI (reg:DI 5 di)
> > (const_int 0 [0]))
> > (expr_list:REG_DEP_TRUE (concat:SI (reg:SI 4 si)
> > (const_int 0 [0]))
> > (nil))) NOTE_INSN_CALL_ARG_LOCATION)
> >
> 
> Huh, this RTX is ignored:

NOTE_INSN_CALL_ARG_LOCATION is fine, even after a REG_NORETURN call.
It is just a way how to pass call argument details to dwarf2out.
If you have a pass after var-tracking, you need to skip over it.

Jakub

Re: [PATCH] PR 54472

[Andrey Belevantsev]

On 30.10.2012 12:09, Andrey Belevantsev wrote:

Hello,

This PR is due to the selective scheduling missing the dependencies with
implicit_sets.  From the sched-deps.c code it looks like implicit sets
generate anti dependencies with either sets, uses or clobbers, so that's
that I've done with the below patch.  Vlad, as it looks you've added
implicit sets, does the above conclusion look correct?  I will commit the
patch then after bootstrapping and testing will complete.



Now committed after offline discussion with Vlad and Alexander.

Andrey

Re: [off-list] Re: [PATCH] Vzeroupper placement/47440

[Uros Bizjak]

On Fri, Nov 9, 2012 at 1:18 PM, Vladimir Yakovlev  wrote:
>> These assert should tell you what is wrong with the control flow.
>> Please look at control_flow_insn_p, which condition returns true.
>
> There is a note after call insn.
>
> (call_insn:TI 908 35558 50534 1681 (call (mem:QI (symbol_ref:DI
> ("_gfortran_stop_string") [flags 0x41]  _gfortran_stop_string>) [0 _gfortran_stop_string S1 A8])
> (const_int 0 [0])) huygens.fppized.f90:190 616 {*call}
>  (expr_list:REG_DEAD (reg:DI 5 di)
> (expr_list:REG_DEAD (reg:SI 4 si)
> (expr_list:REG_NORETURN (const_int 0 [0])
> (nil
> (expr_list:REG_FRAME_RELATED_EXPR (use (reg:DI 5 di))
> (expr_list:REG_BR_PRED (use (reg:SI 4 si))
> (nil
> (note 50534 908 909 1681 (expr_list:REG_DEP_TRUE (concat:DI (reg:DI 5 di)
> (const_int 0 [0]))
> (expr_list:REG_DEP_TRUE (concat:SI (reg:SI 4 si)
> (const_int 0 [0]))
> (nil))) NOTE_INSN_CALL_ARG_LOCATION)
>

Huh, this RTX is ignored:

--cfgrtl.c--
bool
control_flow_insn_p (const_rtx insn)
{
  switch (GET_CODE (insn))
{
case NOTE:
case CODE_LABEL:
case DEBUG_INSN:
  return false;
--cfgrtl.c--

The problem is noreturn call.

BTW: What happens if the new pass is put before pro_and_epilogue pass?

Uros,

Re: [off-list] Re: [PATCH] Vzeroupper placement/47440

[Vladimir Yakovlev]

> These assert should tell you what is wrong with the control flow.
> Please look at control_flow_insn_p, which condition returns true.

There is a note after call insn.

(call_insn:TI 908 35558 50534 1681 (call (mem:QI (symbol_ref:DI
("_gfortran_stop_string") [flags 0x41] ) [0 _gfortran_stop_string S1 A8])
(const_int 0 [0])) huygens.fppized.f90:190 616 {*call}
 (expr_list:REG_DEAD (reg:DI 5 di)
(expr_list:REG_DEAD (reg:SI 4 si)
(expr_list:REG_NORETURN (const_int 0 [0])
(nil
(expr_list:REG_FRAME_RELATED_EXPR (use (reg:DI 5 di))
(expr_list:REG_BR_PRED (use (reg:SI 4 si))
(nil
(note 50534 908 909 1681 (expr_list:REG_DEP_TRUE (concat:DI (reg:DI 5 di)
(const_int 0 [0]))
(expr_list:REG_DEP_TRUE (concat:SI (reg:SI 4 si)
(const_int 0 [0]))
(nil))) NOTE_INSN_CALL_ARG_LOCATION)

> You shouldn't disable commit_edge_insertions, as there is the function
> where vzerouppers are emitted.

I didn;t disable commit_edge_insertions. I only remove call of assert.

2012/11/9 Uros Bizjak :
> On Fri, Nov 9, 2012 at 11:45 AM, Uros Bizjak  wrote:
>> On Fri, Nov 9, 2012 at 11:21 AM, Vladimir Yakovlev  
>> wrote:
>>> I did changes that moves vzeroupper insertion after reload
>>>
>>> 2012-11-09  Vladimir Yakovlev  
>>>
>>> * i386/i386-protos.h (ix86_avx256_optimize_mode_switching): New.
>>> * config/i386/i386.c (ix86_init_machine_status): Deleted
>>> initialization for mode switching.
>>> * i386/i386.h (OPTIMIZE_MODE_SWITCHING1): New.
>>> * mode-switching.c (gate_mode_switching1): New.
>>> (rest_of_handle_mode_switching1): New.
>>> (pass_mode_switching1): New.
>>> * passes.c (init_optimization_passes): New pass 
>>> pass_mode_switching1.
>>> * tree-pass.h (pass_mode_switching1): New.
>>>
>>> But this caused assertion fails in  rtl_verify_flow_info_1 () at 
>>> cfgrtl.c:2291
>>>
>>>   fatal_insn ("flow control insn inside a basic block", x);
>>>
>>> The asserts are called by two calls of mode-switching.c:
>>> commit_edge_insertion and cleanup_cfg. After I commented (see below)
>>> 459.GemsFDTD benchspec passed. Your opinion of the patch and haw we
>>> can do something with asserts.
>
> These assert should tell you what is wrong with the control flow.
> Please look at control_flow_insn_p, which condition returns true. You
> shouldn't disable commit_edge_insertions, as there is the function
> where vzerouppers are emitted.
>
> Uros.

Re: [PR 55238] More careful pass-through handling in find_aggregate_values_for_callers_subset

[Jan Hubicka]

> Hi,
> 
> when writing find_aggregate_values_for_callers_subset, I omitted a
> test that is present in propagate_aggs_accross_jump_function and that
> lead to triggering an assert in the former.
> 
> Fixed by moving the test into a separate predicate function in the
> following patch.
> 
> Bootstrapped and tested on x86_64-linux.  OK for trunk?
MThis patch looks OK.  i will re-test and commit it on the afernoon.

Honza

> 
> Thanks,
> 
> Martin
> 
> 
> 2012-11-08  Martin Jambor  
> 
>   PR tree-optimization/55238
>   * ipa-cp.c (agg_pass_through_permissible_p): New function.
>   (propagate_aggs_accross_jump_function): Use it.
>   (find_aggregate_values_for_callers_subset): Likewise and relax an
>   assert.
> 
>   * testsuite/gcc.dg/torture/pr55238.c: New test.
> 
> Index: src/gcc/ipa-cp.c
> ===
> --- src.orig/gcc/ipa-cp.c
> +++ src/gcc/ipa-cp.c
> @@ -1312,6 +1312,19 @@ merge_aggregate_lattices (struct cgraph_
>return ret;
>  }
>  
> +/* Determine whether there is anything to propagate FROM SRC_PLATS through a
> +   pass-through JFUNC and if so, whether it has conform and conforms to the
> +   rules about propagating values passed by reference.  */
> +
> +static bool
> +agg_pass_through_permissible_p (struct ipcp_param_lattices *src_plats,
> + struct ipa_jump_func *jfunc)
> +{
> +  return src_plats->aggs
> +&& (!src_plats->aggs_by_ref
> + || ipa_get_jf_pass_through_agg_preserved (jfunc));
> +}
> +
>  /* Propagate scalar values across jump function JFUNC that is associated with
> edge CS and put the values into DEST_LAT.  */
>  
> @@ -1333,9 +1346,7 @@ propagate_aggs_accross_jump_function (st
>struct ipcp_param_lattices *src_plats;
>  
>src_plats = ipa_get_parm_lattices (caller_info, src_idx);
> -  if (src_plats->aggs
> -   && (!src_plats->aggs_by_ref
> -   || ipa_get_jf_pass_through_agg_preserved (jfunc)))
> +  if (agg_pass_through_permissible_p (src_plats, jfunc))
>   {
> /* Currently we do not produce clobber aggregate jump
>functions, replace with merging when we do.  */
> @@ -2893,23 +2904,33 @@ find_aggregate_values_for_callers_subset
>  
> if (caller_info->ipcp_orig_node)
>   {
> -   if (!inter)
> - inter = agg_replacements_to_vector (cs->caller, 0);
> -   else
> - intersect_with_agg_replacements (cs->caller, src_idx,
> -  &inter, 0);
> +   struct cgraph_node *orig_node = caller_info->ipcp_orig_node;
> +   struct ipcp_param_lattices *orig_plats;
> +   orig_plats = ipa_get_parm_lattices (IPA_NODE_REF (orig_node),
> +   src_idx);
> +   if (agg_pass_through_permissible_p (orig_plats, jfunc))
> + {
> +   if (!inter)
> + inter = agg_replacements_to_vector (cs->caller, 0);
> +   else
> + intersect_with_agg_replacements (cs->caller, src_idx,
> +  &inter, 0);
> + }
>   }
> else
>   {
> struct ipcp_param_lattices *src_plats;
> src_plats = ipa_get_parm_lattices (caller_info, src_idx);
> -   /* Currently we do not produce clobber aggregate jump
> -  functions, adjust when we do.  */
> -   gcc_checking_assert (!jfunc->agg.items);
> -   if (!inter)
> - inter = copy_plats_to_inter (src_plats, 0);
> -   else
> - intersect_with_plats (src_plats, &inter, 0);
> +   if (agg_pass_through_permissible_p (src_plats, jfunc))
> + {
> +   /* Currently we do not produce clobber aggregate jump
> +  functions, adjust when we do.  */
> +   gcc_checking_assert (!jfunc->agg.items);
> +   if (!inter)
> + inter = copy_plats_to_inter (src_plats, 0);
> +   else
> + intersect_with_plats (src_plats, &inter, 0);
> + }
>   }
>   }
> else if (jfunc->type == IPA_JF_ANCESTOR
> @@ -2933,7 +2954,7 @@ find_aggregate_values_for_callers_subset
> src_plats = ipa_get_parm_lattices (caller_info, src_idx);;
> /* Currently we do not produce clobber aggregate jump
>functions, adjust when we do.  */
> -   gcc_checking_assert (!jfunc->agg.items);
> +   gcc_checking_assert (!src_plats->aggs || !jfunc->agg.items);
> if (!inter)
>   inter = copy_plats_to_inter (src_plats, delta);
> else
> Index: src/gcc/testsuite/gcc.dg/to

Re: [PATCH] Attribute for unused warning for variables of non-trivial types

[Florian Weimer]

On 11/08/2012 08:43 PM, Lubos Lunak wrote:


  The patch implements an attribute for marking types for which gcc cannot on
its own issue warnings about unused variables (e.g. because the ctor is
external), but for which such a warning might be useful anyway (e.g.
std::string).


I'm not sure if the default shouldn't be "warn".  RAII-only classes 
which are used purely for constructor/destructor side effects are pretty 
rare, AFAICT.


To make this useful with containers, we'd need further annotations to 
tell read and write accesses apart.  (A vector might be append-only, and 
the elements might be ignored.)


--
Florian Weimer / Red Hat Product Security Team

[Ada] Minor tweaks in gnat_to_gnu

[Eric Botcazou]

This fixes a couple of cases where gnat_to_gnu would recurse on an empty node 
and removes all traces of SCIL references from gigi.

Tested on x86_64-suse-linux, applied on mainline.


2012-11-09  Eric Botcazou  

* gcc-interface/trans.c (gnat_to_gnu) : Deal
with a missing library unit instead of calling gnat_to_gnu on Emtpy.
Remove last bits of support for SCIL nodes.  Add cases for nodes that
should only be present when annotating types.  Add default abort.


-- 
Eric BotcazouIndex: gcc-interface/trans.c
===
--- gcc-interface/trans.c	(revision 193322)
+++ gcc-interface/trans.c	(working copy)
@@ -6493,7 +6493,13 @@ gnat_to_gnu (Node_Id gnat_node)
 case N_Protected_Body_Stub:
 case N_Task_Body_Stub:
   /* Simply process whatever unit is being inserted.  */
-  gnu_result = gnat_to_gnu (Unit (Library_Unit (gnat_node)));
+  if (Present (Library_Unit (gnat_node)))
+	gnu_result = gnat_to_gnu (Unit (Library_Unit (gnat_node)));
+  else
+	{
+	  gcc_assert (type_annotate_only);
+	  gnu_result = alloc_stmt_list ();
+	}
   break;
 
 case N_Subunit:
@@ -6855,11 +6861,20 @@ gnat_to_gnu (Node_Id gnat_node)
   gnu_result = alloc_stmt_list ();
   break;
 
-default:
-  /* SCIL nodes require no processing for GCC.  Other nodes should only
-	 be present when annotating types.  */
-  gcc_assert (IN (kind, N_SCIL_Node) || type_annotate_only);
+case N_Function_Specification:
+case N_Procedure_Specification:
+case N_Op_Concat:
+case N_Component_Association:
+case N_Protected_Body:
+case N_Task_Body:
+  /* These nodes should only be present when annotating types.  */
+  gcc_assert (type_annotate_only);
   gnu_result = alloc_stmt_list ();
+  break;
+
+default:
+  /* Other nodes are not supposed to reach here.  */
+  gcc_unreachable ();
 }
 
   /* If we pushed the processing of the elaboration routine, pop it back.  */

Fwd: [off-list] Re: [PATCH] Vzeroupper placement/47440

[Vladimir Yakovlev]

-- Forwarded message --
From: Vladimir Yakovlev 
Date: 2012/11/9
Subject: Re: [off-list] Re: [PATCH] Vzeroupper placement/47440
To: Uros Bizjak 
Копия: "H.J. Lu" , Igor Zamyatin 


I did changes that moves vzeroupper insertion after reload

2012-11-09  Vladimir Yakovlev  

* i386/i386-protos.h (ix86_avx256_optimize_mode_switching): New.
* config/i386/i386.c (ix86_init_machine_status): Deleted
initialization for mode switching.
* i386/i386.h (OPTIMIZE_MODE_SWITCHING1): New.
* mode-switching.c (gate_mode_switching1): New.
(rest_of_handle_mode_switching1): New.
(pass_mode_switching1): New.
* passes.c (init_optimization_passes): New pass pass_mode_switching1.
* tree-pass.h (pass_mode_switching1): New.

But this caused assertion fails in  rtl_verify_flow_info_1 () at cfgrtl.c:2291

  fatal_insn ("flow control insn inside a basic block", x);

The asserts are called by two calls of mode-switching.c:
commit_edge_insertion and cleanup_cfg. After I commented (see below)
459.GemsFDTD benchspec passed. Your opinion of the patch and haw we
can do something with asserts.

Regards,
Vladimir

--- a/gcc/mode-switching.c
+++ b/gcc/mode-switching.c
@@ -747,7 +747,7 @@ optimize_mode_switching (void)
 commit_edge_insertions ();

 #if defined (MODE_ENTRY) && defined (MODE_EXIT)
-  cleanup_cfg (CLEANUP_NO_INSN_DEL);
+  /*cleanup_cfg (CLEANUP_NO_INSN_DEL);*/
 #else
   if (!need_commit && !emitted)
 return 0;
--- a/gcc/cfgrtl.c
+++ b/gcc/cfgrtl.c
@@ -1828,7 +1828,7 @@ commit_edge_insertions (void)
   basic_block bb;

 #ifdef ENABLE_CHECKING
-  verify_flow_info ();
+  /*verify_flow_info ();*/
 #endif


2012/11/9 Uros Bizjak :
> On Thu, Nov 8, 2012 at 6:52 PM, Uros Bizjak  wrote:
>
>> Uh, this is spill around call insn, produced by reload.
>>
>> Please compile this code:
>>
>> double test (double a)
>> {
>>   printf ("Hello\n");
>>   return a;
>> }
>>
>> You will get at mode switching:
>>
>> 1 NOTE_INSN_DELETED
>> 4 NOTE_INSN_BASIC_BLOCK
>> 2 r60:DF=xmm0:DF
>>   REG_DEAD: xmm0:DF
>> 3 NOTE_INSN_FUNCTION_BEG
>> 6 di:DI=`*.LC0'
>> 7 call <...>
>>   REG_DEAD: di:DI
>>   REG_UNUSED: ax:SI
>>12 xmm0:DF=r60:DF
>>   REG_DEAD: r60:DF
>>15 use xmm0:DF
>>
>> But reload will insert:
>>
>> 1 NOTE_INSN_DELETED
>> 4 NOTE_INSN_BASIC_BLOCK
>> 2 xmm0:DF=xmm0:DF
>>   REG_DEAD: xmm0:DF
>>18 [sp:DI+0x8]=xmm0:DF
>>   REG_DEAD: xmm0:DF
>> 3 NOTE_INSN_FUNCTION_BEG
>> 6 di:DI=`*.LC0'
>> 7 call <...>
>>   REG_DEAD: di:DI
>>   REG_UNUSED: ax:SI
>>19 xmm0:DF=[sp:DI+0x8]
>>   REG_DEAD: r62:DF
>>12 xmm0:DF=xmm0:DF
>>   REG_DEAD: xmm0:DF
>>15 use xmm0:DF
>>
>> I was not paying attention to this situation.
>
>
> A viable solution to this issue is through machine-reorg function (AKA
> x86_reorg) that would just move vzeroupper to the close proximity to a
> call insn. This would work on non-64bit-MS-ABI targets, where all SSE
> registers are dead at call insn place.
>
> Please note that 64bit-MS-ABI target declares registers xmm6+ as
> call-saved, so they can live over the call. I am not familiar with
> this target, but it looks to me that we have to remove vzeroupper, if
> one or more call-saved SSE registers are live at the call insn place.
>
> Uros.


prvzu.patch
Description: Binary data

[SH] Enable shrink-wrap with reorder_blocks_and_parition

[Christian Bruel]

Hi Kaz,

Now that the cross-jumping problem is fixed since rev #193350, I'd like
to remove this restriction and close PR/54546.

Checked with default sh-sim target_board and
--target_board=sh-sim/-freorder-blocks-and-partition.

Thanks

Christian



2012-11-09  Christian Bruel  

	* config/sh/sh.c (sh_can_use_simple_return_p): Allow with reorder-blocks-and-partition.

Index: config/sh/sh.c
===
--- config/sh/sh.c	(revision 193350)
+++ config/sh/sh.c	(working copy)
@@ -13330,10 +13330,6 @@ sh_can_use_simple_return_p (void)
   if (optimize_function_for_size_p (cfun))
 return false;
 
-  /* Can't optimize CROSSING_JUMPS for now.  */
-  if (flag_reorder_blocks_and_partition)
-return false;
-
   /* Finally, allow for pr save.  */
   d = calc_live_regs (&live_regs_mask);
 

[Darwin] Do not enable -fvar-tracking at -O0

[Eric Botcazou]

Hi,

this is a regression present on the mainline and 4.7 branch.  -fvar-tracking 
is automatically enabled on Darwin when -g is passed, including at -O0.  This 
results in bloated and incomplete debug info.

This comes from a latent issue in the handling of flag_var_tracking_uninit in 
the Darwin back-end, which wasn't updated for the new option machinery.

Tested on x86_64-apple-darwin10.8.0, OK for mainline and 4.7 branch?


2012-11-09  Eric Botcazou  

* common.opt (fvar-tracking-uninit): Document.
* toplev.c (process_options): Fix handling of flag_var_tracking_uninit.
* config/darwin.c (darwin_override_options): Likewise.


-- 
Eric BotcazouIndex: toplev.c
===
--- toplev.c	(revision 193322)
+++ toplev.c	(working copy)
@@ -1416,12 +1416,15 @@ process_options (void)
   /* If the user specifically requested variable tracking with tagging
  uninitialized variables, we need to turn on variable tracking.
  (We already determined above that variable tracking is feasible.)  */
-  if (flag_var_tracking_uninit)
+  if (flag_var_tracking_uninit == 1)
 flag_var_tracking = 1;
 
   if (flag_var_tracking == AUTODETECT_VALUE)
 flag_var_tracking = optimize >= 1;
 
+  if (flag_var_tracking_uninit == AUTODETECT_VALUE)
+flag_var_tracking_uninit = flag_var_tracking;
+
   if (flag_var_tracking_assignments == AUTODETECT_VALUE)
 flag_var_tracking_assignments = flag_var_tracking
   && !(flag_selective_scheduling || flag_selective_scheduling2);
Index: common.opt
===
--- common.opt	(revision 193322)
+++ common.opt	(working copy)
@@ -2189,6 +2189,10 @@ fvar-tracking-assignments-toggle
 Common Report Var(flag_var_tracking_assignments_toggle) Optimization
 Toggle -fvar-tracking-assignments
 
+; Positive if we should track uninitialized variables, negative if
+; we should run the var-tracking pass only to discard debug
+; annotations.  When flag_var_tracking_uninit == AUTODETECT_VALUE it
+; will be set according to flag_var_tracking.
 fvar-tracking-uninit
 Common Report Var(flag_var_tracking_uninit) Optimization
 Perform variable tracking and also tag variables that are uninitialized
Index: config/darwin.c
===
--- config/darwin.c	(revision 193322)
+++ config/darwin.c	(working copy)
@@ -3038,12 +3038,12 @@ darwin_override_options (void)
   darwin_emit_branch_islands = true;
 }
 
-  if (flag_var_tracking
+  if (flag_var_tracking_uninit == 0
   && generating_for_darwin_version >= 9
   && (flag_gtoggle ? (debug_info_level == DINFO_LEVEL_NONE)
   : (debug_info_level >= DINFO_LEVEL_NORMAL))
   && write_symbols == DWARF2_DEBUG)
-flag_var_tracking_uninit = 1;
+flag_var_tracking_uninit = flag_var_tracking;
 
   if (MACHO_DYNAMIC_NO_PIC_P)
 {

Re: [patch] PR middle-end/54385

[Eric Botcazou]

> Updated patch attached, tested on powerpc64-unknown-linux-gnu.
> This is exactly the same patch as before, except that it now uses
> "cleanup_cfg (0)" instead of delete_unreachable_blocks.
> 
> OK for trunk?

OK if you adjust the ChangeLog as well, thanks.

-- 
Eric Botcazou

Patch: Print SImode register names to force addr32 prefix

[H.J. Lu]

Hi,

Since x32 runs in 64-bit mode, for address -0x4300(%rax), hardware
sign-extends displacement from 32-bits to 64-bits and adds it to %rax.
But x32 wants 32-bit -0x4300, not 64-bit -0x4300.  This patch
uses 32-bit registers instead of 64-bit registers when displacement 
< -16*1024*1024.  -16*1024*1024 is used instead of 0 so that we will
still generate -16(%rsp) instead of -16(%esp).

Tested it on Linux/x32.  OK to install?

Thanks.


H.J.
---
2012-11-09  H.J. Lu  

* config/i386/i386.c (ix86_print_operand_address): For x32,
print SImode register names to force addr32 prefix if displacement
< -16*1024*1024.

diff --git a/gcc/config/i386/i386.c b/gcc/config/i386/i386.c
index 78e840b..6d4fbb5 100644
--- a/gcc/config/i386/i386.c
+++ b/gcc/config/i386/i386.c
@@ -14502,6 +14514,17 @@ ix86_print_operand_address (FILE *file, rtx addr)
  gcc_assert (!code);
  code = 'l';
}
+  else if (code == 0
+  && TARGET_X32
+  && disp
+  && CONST_INT_P (disp)
+  && INTVAL (disp) < -16*1024*1024)
+   {
+ /* For x32, print SImode register names to force addr32 prefix
+   if displacement < -16*1024*1024 so that 32-bit displacement
+   isn't sign-extended to 64-bit.  */
+ code = 'k';
+   }
 
   if (ASSEMBLER_DIALECT == ASM_ATT)
{

[x86] Fix ICE with -fstack-check on Linux

[Eric Botcazou]

This is a regression present on the mainline and 4.7 branch.  The compiler 
ICEs on the attached Ada testcase at -O -fstack-check in ix86_expand_prologue:

  gcc_assert (m->fs.sp_offset == frame.stack_pointer_offset);

The problem is that release_scratch_register_on_entry uses a POP instruction 
but doesn't update cfun->machine accordingly.

Tested on x86_64-suse-linux, applied on mainline and 4.7 branch as obvious.


2012-11-09  Eric Botcazou  

* config/i386/i386.c (release_scratch_register_on_entry): Also adjust
sp_offset manually.


2012-11-09  Eric Botcazou  

* gnat.dg/stack_check3.ad[sb]: New test.


-- 
Eric BotcazouIndex: config/i386/i386.c
===
--- config/i386/i386.c	(revision 193322)
+++ config/i386/i386.c	(working copy)
@@ -9462,6 +9462,7 @@ release_scratch_register_on_entry (struc
 {
   if (sr->saved)
 {
+  struct machine_function *m = cfun->machine;
   rtx x, insn = emit_insn (gen_pop (sr->reg));
 
   /* The RTX_FRAME_RELATED_P mechanism doesn't know about pop.  */
@@ -9469,6 +9470,7 @@ release_scratch_register_on_entry (struc
   x = gen_rtx_PLUS (Pmode, stack_pointer_rtx, GEN_INT (UNITS_PER_WORD));
   x = gen_rtx_SET (VOIDmode, stack_pointer_rtx, x);
   add_reg_note (insn, REG_FRAME_RELATED_EXPR, x);
+  m->fs.sp_offset -= UNITS_PER_WORD;
 }
 }
 
-- { dg-do compile }
-- { dg-options "-O -fstack-check" }

package body Stack_Check3 is

  type Int_Arr is array (1 .. 34) of Integer;

  type Rec (D : Boolean := False) is
record
  case D is
when True  => IA : Int_Arr;
when False => null;
  end case;
end record;

  type Rec_Arr is array (1 .. 256) of Rec;

  protected Prot_Arr is
procedure Reset;
  private
A : Rec_Arr;
  end Prot_Arr;

  protected body Prot_Arr is
procedure Reset is
begin
  A := (others => (D => False));
end Reset;
  end Prot_Arr;

  procedure Reset is
  begin
Prot_Arr.Reset;
  end Reset;

end Stack_Check3;
package Stack_Check3 is

  procedure Reset;

end Stack_Check3;

Re: [patch] PR middle-end/54385

[Steven Bosscher]

On Tue, Aug 28, 2012 at 11:54 PM, Eric Botcazou wrote:
>> The CLEANUP_CFG_CHANGED path looks unnecessary, it looks like this is
>> mostly for repairing loops but I don't see a reason for this in
>> postreload (loops have been freed at that point). I could do
>> cleanup_cfg(0) but there shouldn't be much to clean up other than
>> unreachable blocks. I see delete_unreachable_blocks used a lot too.
>> But I have no strong preference either way. Do you think I should use
>> cleanup_cfg instead?
>
> That would be my personal preference, yes.  It was cheap last time I measured
> and I think it's the right abstraction here: if you remove edges from the CFG
> or apply some similar transformation, you need a cleanup_cfg(0) at the end.

Updated patch attached, tested on powerpc64-unknown-linux-gnu.
This is exactly the same patch as before, except that it now uses
"cleanup_cfg (0)" instead of delete_unreachable_blocks.

OK for trunk?

Ciao!
Steven


PR54385.diff
Description: Binary data