On 02/15/12 12:07, Jakub Jelinek wrote:
On Wed, Feb 15, 2012 at 11:59:15AM -0600, Aldy Hernandez wrote:
Hmmm, isn't %K for trees?  We're talking gimple, and FUNCTION_DECLs,
which don't have a TREE_BLOCK, here.

Gimple stmts have gimple_block, I guess you'd need to create some tree
and set its TREE_BLOCK and EXPR_LOCATION from gimple_block/gimple_location.
Or add something similar to %K that would take the same info from gimple
stmt and pass a stmt instead of a tree.

        Jakub

This may not actually work. I am getting a different backtrace depending on whether I use -O0 or -O[123]. With the attached patch I get a proper backtrace with -O0, but none whatsoever with -O1:

#GOOD
houston:/build/t2/gcc$ ./cc1 a.c -fgnu-tm -O0 -quiet -w
In function 'asmfunc',
    inlined from 'f' at a.c:13:10:
a.c:7:3: error: asm not allowed in 'transaction_safe' function

#BAD
houston:/build/t2/gcc$ ./cc1 a.c -fgnu-tm -O1 -quiet -w
a.c: In function 'f':
a.c:7:3: error: asm not allowed in 'transaction_safe' function
houston:/build/t2/gcc$

With -O1, the TREE_BLOCK looks like this:

(gdb) ptg block
BLOCK #3
  SUPERCONTEXT: BLOCK #0
  ABSTRACT_ORIGIN: BLOCK #0

and the BLOCK_SUPERCONTEXT looks like this:

(gdb) ptg block
BLOCK #0
  SUPERCONTEXT: f
  SUBBLOCKS: BLOCK #3

Notice there is no BLOCK_ABSTRACT_ORIGIN in this last one, so we don't print any context.

I would rather have a consistent error message (though possibly confusing), than have an error message that sometimes makes sense.

To make matters worse, I have just noticed that at -O2 and -O3 we inlined all the functions into main, so the GIMPLE_ASM ends up in the transaction itself (see test), not in f(), so the compiler is able to determine we have a GIMPLE_ASM inside a transaction and issues a different error:

houston:/build/t2/gcc$ ./cc1 a.c -fgnu-tm -O2 -quiet -w
a.c:7:3: error: asm not allowed in atomic transaction

Here also we could've used the %K format modifier, if indeed we had full backtrace information. But, like -O1, we don't.

I think I'm back to learning towards rth's suggestion of disabling early inlining.

Thoughts?
Index: testsuite/gcc.dg/tm/pr52141.c
===================================================================
--- testsuite/gcc.dg/tm/pr52141.c       (revision 0)
+++ testsuite/gcc.dg/tm/pr52141.c       (revision 0)
@@ -0,0 +1,24 @@
+/* { dg-do compile } */
+/* { dg-options "-fgnu-tm -O0 -w" } */
+
+__attribute__((always_inline))
+static void asmfunc(void)
+{
+  __asm__ (""); /* { dg-error "asm not allowed in .transaction_safe" } */
+}
+
+__attribute__((transaction_safe))
+static void f(void)
+{
+  asmfunc();
+}
+
+int main()
+{
+  __transaction_atomic {
+    f();
+  }
+  return 0;
+}
+
+/* { dg-message "inlined from \'f\'" "" { target *-*-* } 0 } */
Index: trans-mem.c
===================================================================
--- trans-mem.c (revision 184272)
+++ trans-mem.c (working copy)
@@ -3736,6 +3736,13 @@ ipa_tm_scan_irr_block (basic_block bb)
             assembly statement is not relevant to the transaction
             is to wrap it in a __tm_waiver block.  This is not
             yet implemented, so we can't check for it.  */
+         if (is_tm_safe (current_function_decl))
+           {
+             tree t = build1 (NOP_EXPR, void_type_node, size_zero_node);
+             SET_EXPR_LOCATION (t, gimple_location (stmt));
+             TREE_BLOCK (t) = gimple_block (stmt);
+             error ("%Kasm not allowed in %<transaction_safe%> function", t);
+           }
          return true;
 
        default:

Reply via email to