| Message ID | 564BC70C.70805@linaro.org |
|---|---|
| State | New |
| Headers | show |
On 18/11/15 00:32, Kugan wrote: >> > Hi Ramana, >> > >> > Thanks for the review. I have opened a gcc bug-report for this. I tested >> > the attached patch for arm-none-linux-gnueabihf and >> > arm-none-linux-gnueabi with no new regressions. Is this OK? >> > >> > >> > Thanks, >> > Kugan >> > >> > gcc/ChangeLog: >> > >> > 2015-11-18 Kugan Vivekanandarajah <kuganv@linaro.org> >> > >> > PR target/68390 >> > * config/arm/arm.c (arm_function_ok_for_sibcall): Get function type >> > for indirect function call. >> > >> > gcc/testsuite/ChangeLog: >> > >> > 2015-11-18 Kugan Vivekanandarajah <kuganv@linaro.org> >> > >> > PR target/68390 >> > * gcc.target/arm/PR68390.c: New test. >> > >> > > Hi Ramana, > > With further testing on bare-metal, I found that for the following decl > has to be null for indirect functions. > > if (TARGET_AAPCS_BASED > && arm_abi == ARM_ABI_AAPCS > && decl > && DECL_WEAK (decl)) > return false; Ok .. yes that's right. > > Here is the updated patch and ChangeLog. Sorry for the noise. > > Thanks, > Kugan > > > gcc/ChangeLog: > > 2015-11-18 Kugan Vivekanandarajah <kuganv@linaro.org> > > PR target/68390 > * config/arm/arm.c (arm_function_ok_for_sibcall): Get function type > for indirect function call. > > gcc/testsuite/ChangeLog: > > 2015-11-18 Kugan Vivekanandarajah <kuganv@linaro.org> > > PR target/68390 > * gcc.target/arm/PR68390.c: New test. > s/PR/pr in the test name and put this in gcc.c-torture/execute instead - there is nothing ARM specific about the test. Tests in gcc.target/arm should really only be architecture specific. This isn't. > > > > p.txt > > > diff --git a/gcc/config/arm/arm.c b/gcc/config/arm/arm.c > index a379121..0dae7da 100644 > --- a/gcc/config/arm/arm.c > +++ b/gcc/config/arm/arm.c > @@ -6680,8 +6680,13 @@ arm_function_ok_for_sibcall (tree decl, tree exp) > a VFP register but then need to transfer it to a core > register. */ > rtx a, b; > + tree fn_decl = decl; Call it decl_or_type instead - it's really that ... > > - a = arm_function_value (TREE_TYPE (exp), decl, false); > + /* If it is an indirect function pointer, get the function type. */ > + if (!decl) > + fn_decl = TREE_TYPE (TREE_TYPE (CALL_EXPR_FN (exp))); > + This is probably just my mail client - but please watch out for indentation. > + a = arm_function_value (TREE_TYPE (exp), fn_decl, false); > b = arm_function_value (TREE_TYPE (DECL_RESULT (cfun->decl)), > cfun->decl, false); > if (!rtx_equal_p (a, b)) OK with those changes. Ramana
>> >> gcc/ChangeLog: >> >> 2015-11-18 Kugan Vivekanandarajah <kuganv@linaro.org> >> >> PR target/68390 >> * config/arm/arm.c (arm_function_ok_for_sibcall): Get function type >> for indirect function call. >> >> gcc/testsuite/ChangeLog: >> >> 2015-11-18 Kugan Vivekanandarajah <kuganv@linaro.org> >> >> PR target/68390 >> * gcc.target/arm/PR68390.c: New test. >> > > s/PR/pr in the test name and put this in gcc.c-torture/execute instead - there is nothing ARM specific about the test. Tests in gcc.target/arm should really only be architecture specific. This isn't. > >> >> >> >> p.txt >> >> >> diff --git a/gcc/config/arm/arm.c b/gcc/config/arm/arm.c >> index a379121..0dae7da 100644 >> --- a/gcc/config/arm/arm.c >> +++ b/gcc/config/arm/arm.c >> @@ -6680,8 +6680,13 @@ arm_function_ok_for_sibcall (tree decl, tree exp) >> a VFP register but then need to transfer it to a core >> register. */ >> rtx a, b; >> + tree fn_decl = decl; > > Call it decl_or_type instead - it's really that ... > >> >> - a = arm_function_value (TREE_TYPE (exp), decl, false); >> + /* If it is an indirect function pointer, get the function type. */ >> + if (!decl) >> + fn_decl = TREE_TYPE (TREE_TYPE (CALL_EXPR_FN (exp))); >> + > > This is probably just my mail client - but please watch out for indentation. > >> + a = arm_function_value (TREE_TYPE (exp), fn_decl, false); >> b = arm_function_value (TREE_TYPE (DECL_RESULT (cfun->decl)), >> cfun->decl, false); >> if (!rtx_equal_p (a, b)) > > > OK with those changes. > > Ramana > Hi Ramana, This issue also remains in 4.9 and 5.0 branches. Is this OK to backport to the release branches. Thanks, Kugan
This issue also remains in 4.9 and 5.0 branches. Is this OK to backport to the release branches. Thanks, Kugan On 02/12/15 10:00, Kugan wrote: > >>> >>> gcc/ChangeLog: >>> >>> 2015-11-18 Kugan Vivekanandarajah <kuganv@linaro.org> >>> >>> PR target/68390 >>> * config/arm/arm.c (arm_function_ok_for_sibcall): Get function type >>> for indirect function call. >>> >>> gcc/testsuite/ChangeLog: >>> >>> 2015-11-18 Kugan Vivekanandarajah <kuganv@linaro.org> >>> >>> PR target/68390 >>> * gcc.target/arm/PR68390.c: New test. >>> >> >> s/PR/pr in the test name and put this in gcc.c-torture/execute instead - there is nothing ARM specific about the test. Tests in gcc.target/arm should really only be architecture specific. This isn't. >> >>> >>> >>> >>> p.txt >>> >>> >>> diff --git a/gcc/config/arm/arm.c b/gcc/config/arm/arm.c >>> index a379121..0dae7da 100644 >>> --- a/gcc/config/arm/arm.c >>> +++ b/gcc/config/arm/arm.c >>> @@ -6680,8 +6680,13 @@ arm_function_ok_for_sibcall (tree decl, tree exp) >>> a VFP register but then need to transfer it to a core >>> register. */ >>> rtx a, b; >>> + tree fn_decl = decl; >> >> Call it decl_or_type instead - it's really that ... >> >>> >>> - a = arm_function_value (TREE_TYPE (exp), decl, false); >>> + /* If it is an indirect function pointer, get the function type. */ >>> + if (!decl) >>> + fn_decl = TREE_TYPE (TREE_TYPE (CALL_EXPR_FN (exp))); >>> + >> >> This is probably just my mail client - but please watch out for indentation. >> >>> + a = arm_function_value (TREE_TYPE (exp), fn_decl, false); >>> b = arm_function_value (TREE_TYPE (DECL_RESULT (cfun->decl)), >>> cfun->decl, false); >>> if (!rtx_equal_p (a, b)) >> >> >> OK with those changes. >> >> Ramana >> >
diff --git a/gcc/config/arm/arm.c b/gcc/config/arm/arm.c index a379121..0dae7da 100644 --- a/gcc/config/arm/arm.c +++ b/gcc/config/arm/arm.c @@ -6680,8 +6680,13 @@ arm_function_ok_for_sibcall (tree decl, tree exp) a VFP register but then need to transfer it to a core register. */ rtx a, b; + tree fn_decl = decl; - a = arm_function_value (TREE_TYPE (exp), decl, false); + /* If it is an indirect function pointer, get the function type. */ + if (!decl) + fn_decl = TREE_TYPE (TREE_TYPE (CALL_EXPR_FN (exp))); + + a = arm_function_value (TREE_TYPE (exp), fn_decl, false); b = arm_function_value (TREE_TYPE (DECL_RESULT (cfun->decl)), cfun->decl, false); if (!rtx_equal_p (a, b)) diff --git a/gcc/testsuite/gcc.target/arm/PR68390.c b/gcc/testsuite/gcc.target/arm/PR68390.c index e69de29..86f07fe 100644 --- a/gcc/testsuite/gcc.target/arm/PR68390.c +++ b/gcc/testsuite/gcc.target/arm/PR68390.c @@ -0,0 +1,27 @@ +/* { dg-do run } */ +/* { dg-options "-O2" } */ + +__attribute__ ((noinline)) +double direct(int x, ...) +{ + return x*x; +} + +__attribute__ ((noinline)) +double broken(double (*indirect)(int x, ...), int v) +{ + return indirect(v); +} + +int main () +{ + double d1, d2; + int i = 2; + d1 = broken (direct, i); + if (d1 != i*i) + { + __builtin_abort (); + } + return 0; +} +