[PATCH] D158197: [PowerPC][lld] Account for additional X-Forms -> D-Form/DS-Forms load/stores when relaxing initial-exec to local-exec

[Amy Kwan via Phabricator via llvm-commits]

amyk added inline comments.


================
Comment at: lld/ELF/Arch/PPC64.cpp:846
+// DS-Form instructions.
+unsigned elf::getPPCDSFormOp(unsigned secondaryOp) {
+  switch (secondaryOp) {
----------------
MaskRay wrote:
> nemanjai wrote:
> > I understand the motivation to return both the primary opcode and the last two bits here to differentiate `LWA` from `LD/STD`. However, I am not a fan of the different interface of these very similar and related functions.
> > 
> > Do any users of `getPPCDFormOp()` need the primary opcode to be in the least significant bits? Could we change the interface to return the instruction with the primary opcode in the most significant bits regardless of whether it is `D-Form/DS-Form` or even (in the future) `DQ-Form`?
> `static unsigned getPPCDSFormOp`
> 
> `getPPCDFormOp` is external just because PPC.cpp uses it as well.
@nemanjai That's a good point, since the functions are very related. 

After taking a look, to me, it doesn't appear that any users of `getPPCDFormOp()` require the primary opcode to be in the least significant bits. 

I also tried shifting all of the D-Forms over to the most significant bits like the DS-Form ones. This works, but I'm uncertain if this is the right approach so I was wondering if you have any thoughts regarding this. 




================
Comment at: lld/ELF/Arch/PPC64.cpp:846
+// DS-Form instructions.
+unsigned elf::getPPCDSFormOp(unsigned secondaryOp) {
+  switch (secondaryOp) {
----------------
amyk wrote:
> MaskRay wrote:
> > nemanjai wrote:
> > > I understand the motivation to return both the primary opcode and the last two bits here to differentiate `LWA` from `LD/STD`. However, I am not a fan of the different interface of these very similar and related functions.
> > > 
> > > Do any users of `getPPCDFormOp()` need the primary opcode to be in the least significant bits? Could we change the interface to return the instruction with the primary opcode in the most significant bits regardless of whether it is `D-Form/DS-Form` or even (in the future) `DQ-Form`?
> > `static unsigned getPPCDSFormOp`
> > 
> > `getPPCDFormOp` is external just because PPC.cpp uses it as well.
> @nemanjai That's a good point, since the functions are very related. 
> 
> After taking a look, to me, it doesn't appear that any users of `getPPCDFormOp()` require the primary opcode to be in the least significant bits. 
> 
> I also tried shifting all of the D-Forms over to the most significant bits like the DS-Form ones. This works, but I'm uncertain if this is the right approach so I was wondering if you have any thoughts regarding this. 
> 
> 
@MaskRay I might be misunderstanding something, but I am also using `getPPCDSFormOp()` in `PPC.cpp`, as well. 
Doesn't this mean this function also needs to be external?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D158197/new/

https://reviews.llvm.org/D158197