please the find test case in the attachment. It shows the issue in
google-4_6 branch
-c -O2 -fno-strict-aliasing ss.C -fdump-rtl-expand-all
in the rtl-expand dump, trianglevertices and one the gtest_ar are in
the same partition.
the problem is found in arm compiler, but we manager to reproduce in x86.
-Rong
On Tue, Apr 24, 2012 at 3:02 PM, Andrew Pinski <pins...@gmail.com> wrote:
> On Tue, Apr 24, 2012 at 2:57 PM, Rong Xu <x...@google.com> wrote:
>> Hi,
>>
>> In split_function() (ipa-split.c), for the newly created call stmt,
>> its block is set to the outermost scope, i.e.
>> DECL_INITIAL(current_function_decl). When we inline this
>> partially outlined function, we create the new block based on the
>> block for the call stmt (in expand_call_inline()).
>> So the new block for the inlined body is in
>> parallel to the function top level scope. This bad block structure will
>> late result in a bad stack layout.
>
> We do not depend on the block structure any more when dealing with
> stack layout for variables in GCC 4.7.0 and above. I am not saying
> your patch is incorrect or not needed. Just it will not have an
> effect on variable stack layout.
>
> Did you have a testcase for the bad stack layout issue? Or was it too
> hard to produce one because the size matters?
>
>
>>
>> This patch fixes the issue by setting the correct block number. It
>> starts with the split_point entry bb to find the block stmt in the
>> outlined region. The entry_bb maybe an empty BB so we need to follow
>> the CFG until we find a non-empty bb.
>>
>> My early patch tried to use the block info from the first non-empty
>> bb in the outlined regision. But that failed bootstrap as some of the
>> stmts (assignment stmt) do not have the block info (bug?). So in this
>> patch I traverse all the stmts in the bb until getting the block info.
>>
>> Tested with gcc bootstap.
>
> On which target?
>
> Thanks,
> Andrew Pinski
>
>>
>> Thanks,
>>
>> 2012-04-24 Rong Xu <x...@google.com>
>>
>> * ipa-split.c (split_function): set the correct block for the
>> call statement.
>>
>> Index: ipa-split.c
>> ===================================================================
>> --- ipa-split.c (revision 186634)
>> +++ ipa-split.c (working copy)
>> @@ -948,7 +948,7 @@
>> int num = 0;
>> struct cgraph_node *node, *cur_node = cgraph_node (current_function_decl);
>> basic_block return_bb = find_return_bb ();
>> - basic_block call_bb;
>> + basic_block call_bb, bb;
>> gimple_stmt_iterator gsi;
>> gimple call;
>> edge e;
>> @@ -958,6 +958,7 @@
>> gimple last_stmt = NULL;
>> unsigned int i;
>> tree arg;
>> + tree split_loc_block = NULL;
>>
>> if (dump_file)
>> {
>> @@ -1072,6 +1073,33 @@
>> }
>> }
>>
>> + /* Find the block location of the split region. */
>> + bb = split_point->entry_bb;
>> + do
>> + {
>> + bool found = false;
>> +
>> + for (gsi = gsi_start_bb (bb); !gsi_end_p (gsi); gsi_next (&gsi))
>> + {
>> + if (is_gimple_debug(gsi_stmt(gsi)))
>> + continue;
>> + if ((split_loc_block = gimple_block (gsi_stmt (gsi))))
>> + {
>> + found = true;
>> + break;
>> + }
>> + }
>> + if (found)
>> + break;
>> +
>> + /* If we reach here, this bb should be an empty unconditional
>> + or fall-throuth branch. We continue with the succ bb. */
>> + bb = single_succ (bb);
>> + }
>> + while (bb && bitmap_bit_p (split_point->split_bbs, bb->index));
>> +
>> + gcc_assert (split_loc_block);
>> +
>> /* Now create the actual clone. */
>> rebuild_cgraph_edges ();
>> node = cgraph_function_versioning (cur_node, NULL, NULL, args_to_skip,
>> @@ -1115,7 +1143,7 @@
>> VEC_replace (tree, args_to_pass, i, arg);
>> }
>> call = gimple_build_call_vec (node->decl, args_to_pass);
>> - gimple_set_block (call, DECL_INITIAL (current_function_decl));
>> + gimple_set_block (call, split_loc_block);
>>
>> /* We avoid address being taken on any variable used by split part,
>> so return slot optimization is always possible. Moreover this is
>>
>> --
>> This patch is available for review at http://codereview.appspot.com/6111050
namespace testing
{
class Test
{
virtual void TestBody() = 0;
};
class AssertionResult
{
public:
AssertionResult(const AssertionResult& other);
operator bool() const { return success_; }
private:
bool success_;
void operator=(AssertionResult const &);
};
namespace internal
{
class TestFactoryBase
{
public:
virtual ::testing::Test* CreateTest() = 0;
};
template <class TestClass>
class TestFactoryImpl : public TestFactoryBase
{
public:
virtual ::testing::Test* CreateTest() { return new TestClass; }
};
char* MakeAndRegisterTestInfo2(
const char* test_case_name,
TestFactoryBase* factory);
AssertionResult CmpHelperEQ(const char* expected_expression,
const char* actual_expression,
long long expected,
long long actual);
AssertionResult CmpHelperNE( const char* expr, const char* expr2,
long long val1, long long val2);
template <bool lhs_is_null_literal>
class EqHelper
{
public:
template <typename T1, typename T2>
static AssertionResult Compare(const char* expected_expression,
const char* actual_expression,
const T1& expected,
const T2& actual)
{
return CmpHelperEQ(expected_expression, actual_expression, expected,
actual);
}
static AssertionResult Compare(const char* expected_expression,
const char* actual_expression,
long long expected,
long long actual)
{
return CmpHelperEQ(expected_expression, actual_expression, expected,
actual);
}
};
class Secret;
template<bool> struct EnableIf;
template<> struct EnableIf<true> { typedef void type; };
template <bool bool_value>
struct bool_constant
{
typedef bool_constant<bool_value> type;
static const bool value = bool_value;
};
template <bool bool_value> const bool bool_constant<bool_value>::value;
typedef bool_constant<false> false_type;
typedef bool_constant<true> true_type;
template <typename T>
struct is_pointer : public false_type {};
template <typename T>
struct is_pointer<T*> : public true_type {};
template <>
class EqHelper<true>
{
public:
template <typename T1, typename T2>
static AssertionResult Compare(
const char* expected_expression,
const char* actual_expression,
const T1& expected,
const T2& actual,
typename EnableIf<!is_pointer<T2>::value>::type* = 0)
{
return CmpHelperEQ(expected_expression, actual_expression, expected,
actual);
}
template <typename T>
static AssertionResult Compare(
const char* expected_expression,
const char* actual_expression,
Secret* ,
T* actual)
{
return CmpHelperEQ(expected_expression, actual_expression,
static_cast<T*>(__null), actual);
}
};
char IsNullLiteralHelper(Secret* p);
}
}
extern "C"
{
__attribute__((visibility("default"))) unsigned int glGetError(void);
__attribute__((visibility("default"))) void glVertexAttribPointer(
unsigned int indx, int size, unsigned int type, bool normalized,
int stride, const void* ptr);
}
typedef unsigned int GLenum;
class SGL : public ::testing::Test
{
protected:
void drawTexture()
{
const float triangleVertices[] =
{
-1.0f, 1.0f, -1.0f, -1.0f, 1.0f, -1.0f, 1.0f, 1.0f };
glVertexAttribPointer(mPositionHandle, 2, 0x1406, 0, 0, triangleVertices);
if (const ::testing::AssertionResult gtest_ar = (::testing::internal:: EqHelper<(sizeof(::testing::internal::IsNullLiteralHelper(GLenum(0))) == 1)>::Compare("GLenum(0)", "glGetError()", GLenum(0), glGetError())))
;
else
return;
if (const ::testing::AssertionResult gtest_ar = (::testing::internal:: EqHelper<(sizeof(::testing::internal::IsNullLiteralHelper(GLenum(0))) == 1)>::Compare("GLenum(0)", "glGetError()", GLenum(0), glGetError())))
;
else
return;
}
int mPositionHandle;
};
class SGL_1_Test : public SGL
{
private:
virtual void TestBody()
{
drawTexture();
}
};
class SGL_2_Test : public SGL
{
private:
virtual void TestBody()
{
mPositionHandle = 12;
drawTexture();
}
static const char* test_info_;
};
const char* SGL_2_Test ::test_info_ =
::testing::internal::MakeAndRegisterTestInfo2(
"SGL_2_Test",
new ::testing::internal::TestFactoryImpl<SGL_2_Test>);
