[PATCH] D133464: [WIP][BPF] Add sext load instructions

[Andrii Nakryiko via Phabricator via llvm-commits]

anakryiko added inline comments.


================
Comment at: llvm/lib/Target/BPF/BPFInstrInfo.td:469
+  def LDHS : LOADi64<BPF_H, BPF_MEM_SEXT, "s16", sextloadi16>;
+  def LDBS : LOADi64<BPF_B, BPF_MEM_SEXT, "s8",  sextloadi8>;
 }
----------------
anakryiko wrote:
> ast wrote:
> > anakryiko wrote:
> > > anakryiko wrote:
> > > > ast wrote:
> > > > > I believe x86 and arm64 have instructions that sign extends one register into another,
> > > > > but not insns that sign extend during the load.
> > > > > Such insns can be composed better to in case s32 came as a return value from a call.
> > > > > Have you considered going that route?
> > > > one of the motivations for this instruction was to allow libbpf to automatically change load instruction to a proper size of the field (https://github.com/libbpf/libbpf/blob/master/src/relo_core.c#L930-L940). This works currently only for unsigned fields. If we add some instruction which would have to be called after load, that will not help in this case. So maybe let's add both? Register-to-register would help with calls, load with sign extension will help CO-RE and generally signed loads?
> > > for call optimization, maybe having a family of calls that would perform desired size adjustment (32-bit to 64-bit) and optionally sign extension is another improvement?
> > > 
> > > E.g., what we have as "call" today would be basically 64-bit unsigned call, but we can have call{8,16,32,64}{s,u}, where call16s means that callee function is returning short in r0 and we request that r0 is sign-extended so it can be used as s64.
> > > 
> > > Just a thought.
> > calls don't have such attrs. that's no go.
> > I prefer to avoid new loads, since they don't map to insns directly.
> > libbpf should be able to insert an insn. It can adjust offsets already. Not that hard.
> Understood about call, np. And while you are technically correct about inserting instructions, that's a major PITA, so I'm not so keen about that.
> 
> But then re: x86 not having sign-extending loads. Isn't movsx exactly that (https://www.felixcloutier.com/x86/movsx:movsxd). Not an assembly expert, but seems like source can be a memory location, so that's exactly 1:1 mapping?
> 
arm64 seems to have something like this as well? E.g., LDRSB (https://developer.arm.com/documentation/102374/0100/Loads-and-stores---zero-and-sign-extension)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133464