Re: [Bug target/90513] asm thunks do not work on PowerPC64/VxWorks (kernel mode)

[Segher Boessenkool]

On Tue, Jun 11, 2019 at 10:25:58PM +0530, Umesh Kalappa wrote:
> We would like to know comments on the  below  proposed change ?

As I said in the PR, this is not a proper patch.  Also, like Eric says
there, it does not do the right thing for many configurations.

Fix the loader, instead?


Segher

[Bug target/90513] asm thunks do not work on PowerPC64/VxWorks (kernel mode)

[segher at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=90513

--- Comment #19 from Segher Boessenkool  ---
Also, please post proper patches, to the gcc-patches mailing list, as
detailed in , if you want those
patches to be approved.

[Bug target/90513] asm thunks do not work on PowerPC64/VxWorks (kernel mode)

[ebotcazou at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=90513

--- Comment #18 from Eric Botcazou  ---
> is that ok to commit and we regressed for powerpc and no-regress found .

But this will pessimize e.g. VxWorks in RTP mode which works OK, so no.

Re: [Bug target/90513] asm thunks do not work on PowerPC64/VxWorks (kernel mode)

[Umesh Kalappa]

Hi Segher ,

We would like to know comments on the  below  proposed change ?

Thank you
~Umesh

On Tue, Jun 11, 2019 at 10:23 PM umesh.kalappa0 at gmail dot com
 wrote:
>
> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=90513
>
> --- Comment #17 from Umesh Kalappa  ---
> the following change
>
> #undef TARGET_ASM_CAN_OUTPUT_MI_THUNK
> #define TARGET_ASM_CAN_OUTPUT_MI_THUNK rs6000_can_output_mi_thunk
>
> /* Return true if rs6000_output_mi_thunk would be able to output the
>assembler code for the thunk function specified by the arguments
>it is passed, and false otherwise.  */
>
> static bool
> rs6000_can_output_mi_thunk (const_tree, HOST_WIDE_INT, HOST_WIDE_INT,
> const_tree)
> {
>   if (rs6000_default_long_calls)
> return false;
>
>   /* The loader neither creates the glue code sequence that loads r12 nor uses
>  the local entry point for the sibcall's target in the ELFv2 ABI.  */
>   return DEFAULT_ABI != ABI_ELFv2;
> }
>
> when we have longcall enabled ,we fall through regular asm thunk generation
> like above .
>
> is that ok to commit and we regressed for powerpc and no-regress found .

[Bug target/90513] asm thunks do not work on PowerPC64/VxWorks (kernel mode)

[umesh.kalappa0 at gmail dot com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=90513

--- Comment #17 from Umesh Kalappa  ---
the following change 

#undef TARGET_ASM_CAN_OUTPUT_MI_THUNK
#define TARGET_ASM_CAN_OUTPUT_MI_THUNK rs6000_can_output_mi_thunk

/* Return true if rs6000_output_mi_thunk would be able to output the
   assembler code for the thunk function specified by the arguments
   it is passed, and false otherwise.  */

static bool
rs6000_can_output_mi_thunk (const_tree, HOST_WIDE_INT, HOST_WIDE_INT,
const_tree)
{
  if (rs6000_default_long_calls)
return false;

  /* The loader neither creates the glue code sequence that loads r12 nor uses
 the local entry point for the sibcall's target in the ELFv2 ABI.  */
  return DEFAULT_ABI != ABI_ELFv2;
}

when we have longcall enabled ,we fall through regular asm thunk generation
like above .

is that ok to commit and we regressed for powerpc and no-regress found .

[Bug target/90513] asm thunks do not work on PowerPC64/VxWorks (kernel mode)

[ebotcazou at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=90513

--- Comment #16 from Eric Botcazou  ---
> you mean ,the target-independent code in the C++ front end will generate a
> less efficient heavyweight thunk that calls function instead of jumping to
> it ?

The cgraphunit code (cgraph_node::expand_thunk).  I agree that it's suboptimal
but this can often be recouped by means of inlining.

[Bug target/90513] asm thunks do not work on PowerPC64/VxWorks (kernel mode)

[umesh.kalappa0 at gmail dot com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=90513

--- Comment #15 from Umesh Kalappa  ---
>>Yes, but just use a regular thunk.
you mean ,the target-independent code in the C++ front end will generate a less
efficient heavyweight thunk that calls function instead of jumping to it ?

[Bug target/90513] asm thunks do not work on PowerPC64/VxWorks (kernel mode)

[ebotcazou at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=90513

--- Comment #14 from Eric Botcazou  ---
> thunk required to adjust the this pointer (in c++ cases)

Yes, but just use a regular thunk.

[Bug target/90513] asm thunks do not work on PowerPC64/VxWorks (kernel mode)

[umesh.kalappa0 at gmail dot com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=90513

--- Comment #13 from Umesh Kalappa  ---
>>There is no point in emitting an asm thunk if you use a long call though.
thunk required to adjust the this pointer (in c++ cases)

[Bug target/90513] asm thunks do not work on PowerPC64/VxWorks (kernel mode)

[ebotcazou at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=90513

--- Comment #12 from Eric Botcazou  ---
> for our kernel module we always pass option "-mlongcall" and  we believe
> that ,the asm thunk should generate the long call  here (through call r12 in
> this case)  and we can fix the compiler here to do so (more conservative and
> un-optimial for now).

There is no point in emitting an asm thunk if you use a long call though.

[Bug target/90513] asm thunks do not work on PowerPC64/VxWorks (kernel mode)

[umesh.kalappa0 at gmail dot com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=90513

--- Comment #11 from Umesh Kalappa  ---
Segher and Alan ,
for our kernel module we always pass option "-mlongcall" and  we believe that
,the asm thunk should generate the long call  here (through call r12 in this
case)  and we can fix the compiler here to do so (more conservative and
un-optimial for now).

[Bug target/90513] asm thunks do not work on PowerPC64/VxWorks (kernel mode)

[amodra at gmail dot com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=90513

--- Comment #10 from Alan Modra  ---
Yes, just like the function _ZN12Intermediate1vEv.

From here:
.set.LTHUNK0,_ZN12Intermediate1vEv

[Bug target/90513] asm thunks do not work on PowerPC64/VxWorks (kernel mode)

[segher at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=90513

--- Comment #9 from Segher Boessenkool  ---
With a local entry offset?  Do you mean it has non-zero top three bits of
st_other?

[Bug target/90513] asm thunks do not work on PowerPC64/VxWorks (kernel mode)

[amodra at gmail dot com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=90513

--- Comment #8 from Alan Modra  ---
Oh, and .LTHUNK0 is a function symbol with a local entry offset..

[Bug target/90513] asm thunks do not work on PowerPC64/VxWorks (kernel mode)

[amodra at gmail dot com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=90513

Alan Modra  changed:

   What|Removed |Added

 CC||amodra at gmail dot com

--- Comment #7 from Alan Modra  ---
> The compiler should not require the loader to do this.

Nope.  The compiler *does* require a linker to this.  eg. compile this:

#include 

void __attribute__ ((noclone, noinline))
print (const char *str)
{
  puts (str);
}

int
main ()
{
  print ("Hello");
  return 0;
}

The call to "print" must be adjusted to go to the local entry point.

If you aren't using a final link stage such as when building kernel modules,
then the module loader needs to do the same as the linker.

[Bug target/90513] asm thunks do not work on PowerPC64/VxWorks (kernel mode)

[segher at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=90513

--- Comment #6 from Segher Boessenkool  ---
Confirmed.  We have for the thunk

.set.LTHUNK0,_ZN12Intermediate1vEv


.align 2
.p2align 4,,15
.globl _ZThn8_N12Intermediate1vEv
.type   _ZThn8_N12Intermediate1vEv, @function
_ZThn8_N12Intermediate1vEv:
.LFB27:
.cfi_startproc
.LCF2:
0:  addis 2,12,.TOC.-.LCF2@ha
addi 2,2,.TOC.-.LCF2@l
.localentry _ZThn8_N12Intermediate1vEv,.-_ZThn8_N12Intermediate1vEv
addi 3,3,-8
b .LTHUNK0
.cfi_endproc
.LFE27:
.size   _ZThn8_N12Intermediate1vEv,.-_ZThn8_N12Intermediate1vEv


so this will not work unless the jump is optimised by the loader to jump to the
local entry point.  The compiler should not require the loader to do this.

[Bug target/90513] asm thunks do not work on PowerPC64/VxWorks (kernel mode)

[ebotcazou at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=90513

--- Comment #5 from Eric Botcazou  ---
Created attachment 46370
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=46370=edit
Fix or workaound

Tested on various PowerPC ports.

[Bug target/90513] asm thunks do not work on PowerPC64/VxWorks (kernel mode)

[ebotcazou at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=90513

Eric Botcazou  changed:

   What|Removed |Added

 Target|powerpc64le-*-* |powerpc64-wrs-vxworks
 Status|UNCONFIRMED |NEW
   Last reconfirmed||2019-05-17
Summary|powerpcelfv2 :R2 is not |asm thunks do not work on
   |updated to the  TOC base .  |PowerPC64/VxWorks (kernel
   ||mode)
 Ever confirmed|0   |1