On 02/20/12 03:56, Richard Guenther wrote:
On Thu, Feb 16, 2012 at 7:26 PM, Aldy Hernandez<al...@redhat.com> wrote:
On 02/16/12 12:04, Jakub Jelinek wrote:
On Thu, Feb 16, 2012 at 11:46:33AM -0600, Aldy Hernandez wrote:
#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$
Even with -O1 -g ? With -g0 we prune BLOCKs, so the backtraces
don't work well.
Well with -O1 -g the backtrace works for this particular error. But with
-O2 -g, we trigger another error "asm not allowed in atomic transaction",
and no amount of %K fiddling will get me a backtrace.
But even so, it doesn't seem right to expect users to get a correct error
message only with -g, does it?
Well, you can't expect diagnostics to be perfect with inlining in place.
But you can try improving things by looking at what blocks tree-ssa-live.c
prunes.
Disabling early inlining for TM is a hack, I don't think you want to make
people pay the optimizing penalty just because of asm diagnostics.
Fair enough.
So back to my original patch, but with %K which makes it much more
readable with -O -g or with no optimization.
I checked and at -O[23], the error message is a different one (and not
an ICE thankfully), and has been suffering from the same lack of context
from many eons back, so no regressions there.
This patch fixes the ICE in the PR, and since the function attribute in
the PR is always_inline, it displays a very informative message with
either -O0 or -O -g.
So this patch fixes the PR in its entirety.
OK?
PR middle-end/52141
* trans-mem.c (ipa_tm_scan_irr_block): Error out on GIMPLE_ASM's
in a transaction safe function.
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:
