| Message ID | 50BFB764.6050303@gmail.com |
|---|---|
| State | New |
| Headers | show |
On 2012-12-05 15:06, Aldy Hernandez wrote: > PR rtl-optimization/51771 > Revert: > * builtin-attrs.def (ATTR_RETURNS_TWICE, ATTR_TM_NOTHROW_RT_LIST): New. > * gtm-builtins.def (BUILT_IN_TM_START): Add returns-twice attrib. Are we absolutely sure that the rtl edges are fixed? Fixing only the gimple level edges doesn't really help the regsister allocator. r~
On 12/07/12 14:04, Richard Henderson wrote: > On 2012-12-05 15:06, Aldy Hernandez wrote: >> PR rtl-optimization/51771 >> Revert: >> * builtin-attrs.def (ATTR_RETURNS_TWICE, ATTR_TM_NOTHROW_RT_LIST): New. >> * gtm-builtins.def (BUILT_IN_TM_START): Add returns-twice attrib. > Are we absolutely sure that the rtl edges are fixed? > Fixing only the gimple level edges doesn't really help the regsister allocator. I'm actually sure that the rtl edges are NOT fixed in the rtl :). All the back edges are removed prior to generating RTL in gimple_expand_cfg(), seemingly by design: /* At the moment not all abnormal edges match the RTL representation. It is safe to remove them here as find_many_sub_basic_blocks will rediscover them. In the future we should get this fixed properly. */ What I was assuming was that the returns-twice kludge was papering over some other issue, because we were never sure that the lack of edges was actually causing the problem in the PR. I can't find any tests (internal or external) that need the returns-twice hack. I am hoping that by removing this hack we can find actual problems. However, if you believe the RTL edge dropping in gimple_expand_cfg() is problematic, I can look into fixing this, though it would be nice to have a test I can work off of. Aldy
On 2012-12-07 14:11, Aldy Hernandez wrote: > I am hoping that by removing this hack we can find actual problems. > > However, if you believe the RTL edge dropping in gimple_expand_cfg() is problematic, I can look into fixing this, though it would be nice to have a test I can work off of. Lets leave this for 4.9 work then. r~
diff --git a/gcc/builtin-attrs.def b/gcc/builtin-attrs.def index 545b6fd..d149e26 100644 --- a/gcc/builtin-attrs.def +++ b/gcc/builtin-attrs.def @@ -107,7 +107,6 @@ DEF_ATTR_IDENT (ATTR_STRFTIME, "strftime") DEF_ATTR_IDENT (ATTR_TYPEGENERIC, "type generic") DEF_ATTR_IDENT (ATTR_TM_REGPARM, "*tm regparm") DEF_ATTR_IDENT (ATTR_TM_TMPURE, "transaction_pure") -DEF_ATTR_IDENT (ATTR_RETURNS_TWICE, "returns_twice") DEF_ATTR_TREE_LIST (ATTR_NOVOPS_LIST, ATTR_NOVOPS, ATTR_NULL, ATTR_NULL) @@ -253,8 +252,6 @@ DEF_ATTR_TREE_LIST (ATTR_TM_NORETURN_NOTHROW_LIST, ATTR_TM_REGPARM, ATTR_NULL, ATTR_NORETURN_NOTHROW_LIST) DEF_ATTR_TREE_LIST (ATTR_TM_CONST_NOTHROW_LIST, ATTR_TM_REGPARM, ATTR_NULL, ATTR_CONST_NOTHROW_LIST) -DEF_ATTR_TREE_LIST (ATTR_TM_NOTHROW_RT_LIST, - ATTR_RETURNS_TWICE, ATTR_NULL, ATTR_TM_NOTHROW_LIST) /* Same attributes used for BUILT_IN_MALLOC except with TM_PURE thrown in. */ DEF_ATTR_TREE_LIST (ATTR_TMPURE_MALLOC_NOTHROW_LIST, diff --git a/gcc/gtm-builtins.def b/gcc/gtm-builtins.def index 171019e..7dcad03 100644 --- a/gcc/gtm-builtins.def +++ b/gcc/gtm-builtins.def @@ -1,5 +1,5 @@ DEF_TM_BUILTIN (BUILT_IN_TM_START, "_ITM_beginTransaction", - BT_FN_UINT32_UINT32_VAR, ATTR_TM_NOTHROW_RT_LIST) + BT_FN_UINT32_UINT32_VAR, ATTR_TM_NOTHROW_LIST) DEF_TM_BUILTIN (BUILT_IN_TM_COMMIT, "_ITM_commitTransaction", BT_FN_VOID, ATTR_TM_NOTHROW_LIST)
As mentioned in the PR, this seems to no longer be an issue after we redesigned the TM abnormal edges (in the uninstrumented code path patchset). Tested with the STAMP benchmark for both 1 and 4 threads, as well as with the GCC regression suite. OK for trunk? commit 462e1c25a81bfb575a9113e4d117c5972c827715 Author: Aldy Hernandez <aldyh@redhat.com> Date: Mon Dec 3 13:03:16 2012 -0600 PR rtl-optimization/51771 Revert: * builtin-attrs.def (ATTR_RETURNS_TWICE, ATTR_TM_NOTHROW_RT_LIST): New. * gtm-builtins.def (BUILT_IN_TM_START): Add returns-twice attrib.