All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Jose E. Marchesi" <jose.marchesi@oracle.com>
To: Alexei Starovoitov <alexei.starovoitov@gmail.com>
Cc: Yonghong Song <yonghong.song@linux.dev>,
	Eddy Z <eddyz87@gmail.com>, Tao Lyu <tao.lyu@epfl.ch>,
	Andrii Nakryiko <andrii@kernel.org>,
	Alexei Starovoitov <ast@kernel.org>, bpf <bpf@vger.kernel.org>,
	Daniel Borkmann <daniel@iogearbox.net>,
	Hao Luo <haoluo@google.com>,
	Martin KaFai Lau <martin.lau@linux.dev>,
	mathias.payer@nebelwelt.net, meng.xu.cs@uwaterloo.ca,
	sanidhya.kashyap@epfl.ch, Song Liu <song@kernel.org>
Subject: Re: [PATCH] C inlined assembly for reproducing max<min
Date: Wed, 22 Nov 2023 19:37:44 +0100	[thread overview]
Message-ID: <874jhdk51j.fsf@oracle.com> (raw)
In-Reply-To: <CAADnVQJqmpSoABqd-dCQBU2ExiPda1mHz2pKHv2jzpSMYFMeqQ@mail.gmail.com> (Alexei Starovoitov's message of "Wed, 22 Nov 2023 10:15:55 -0800")


> On Wed, Nov 22, 2023 at 10:08 AM Yonghong Song <yonghong.song@linux.dev> wrote:
>>
>> > +SEC("?tc")
>> > +__log_level(2)
>> > +int test_verifier_range(void)
>> > +{
>> > +    asm volatile (
>> > +        "r5 = 100; \
>> > +        r5 /= 3; \
>> > +        w5 >>= 7; \
>> > +        r5 &= -386969681; \
>> > +        r5 -= -884670597; \
>> > +        w0 = w5; \
>> > +        if w0 & 0x894b6a55 goto +2; \
>>
>> So actually it is 'if w0 & 0x894b6a55 goto +2' failed
>> the compilation.
>>
>> Indeed, the above operation is not supported in llvm.
>> See
>>    https://github.com/llvm/llvm-project/blob/main/llvm/lib/Target/BPF/BPFInstrFormats.td#L62-L74
>> the missing BPFJumpOp<0x4> which corresponds to JSET.
>>
>> The following llvm patch (on top of llvm-project main branch):
>>
>> diff --git a/llvm/lib/Target/BPF/BPFInstrFormats.td b/llvm/lib/Target/BPF/BPFInstrFormats.td
>> index 841d97efc01c..6ed83d877ac0 100644
>> --- a/llvm/lib/Target/BPF/BPFInstrFormats.td
>> +++ b/llvm/lib/Target/BPF/BPFInstrFormats.td
>> @@ -63,6 +63,7 @@ def BPF_JA   : BPFJumpOp<0x0>;
>>   def BPF_JEQ  : BPFJumpOp<0x1>;
>>   def BPF_JGT  : BPFJumpOp<0x2>;
>>   def BPF_JGE  : BPFJumpOp<0x3>;
>> +def BPF_JSET : BPFJumpOp<0x4>;
>>   def BPF_JNE  : BPFJumpOp<0x5>;
>>   def BPF_JSGT : BPFJumpOp<0x6>;
>>   def BPF_JSGE : BPFJumpOp<0x7>;
>> diff --git a/llvm/lib/Target/BPF/BPFInstrInfo.td b/llvm/lib/Target/BPF/BPFInstrInfo.td
>> index 305cbbd34d27..9e75f35efe70 100644
>> --- a/llvm/lib/Target/BPF/BPFInstrInfo.td
>> +++ b/llvm/lib/Target/BPF/BPFInstrInfo.td
>> @@ -246,6 +246,70 @@ class JMP_RI_32<BPFJumpOp Opc, string OpcodeStr, PatLeaf Cond>
>>     let BPFClass = BPF_JMP32;
>>   }
>>
>> +class JSET_RR<string OpcodeStr>
>> +    : TYPE_ALU_JMP<BPF_JSET.Value, BPF_X.Value,
>> +                   (outs),
>> +                   (ins GPR:$dst, GPR:$src, brtarget:$BrDst),
>> +                   "if $dst "#OpcodeStr#" $src goto $BrDst",
>> +                   []> {
>> +  bits<4> dst;
>> +  bits<4> src;
>> +  bits<16> BrDst;
>> +
>> +  let Inst{55-52} = src;
>> +  let Inst{51-48} = dst;
>> +  let Inst{47-32} = BrDst;
>> +  let BPFClass = BPF_JMP;
>> +}
>> +
>> +class JSET_RI<string OpcodeStr>
>> +    : TYPE_ALU_JMP<BPF_JSET.Value, BPF_K.Value,
>> +                   (outs),
>> +                   (ins GPR:$dst, i64imm:$imm, brtarget:$BrDst),
>> +                   "if $dst "#OpcodeStr#" $imm goto $BrDst",
>> +                   []> {
>> +  bits<4> dst;
>> +  bits<16> BrDst;
>> +  bits<32> imm;
>> +
>> +  let Inst{51-48} = dst;
>> +  let Inst{47-32} = BrDst;
>> +  let Inst{31-0} = imm;
>> +  let BPFClass = BPF_JMP;
>> +}
>> +
>> +class JSET_RR_32<string OpcodeStr>
>> +    : TYPE_ALU_JMP<BPF_JSET.Value, BPF_X.Value,
>> +                   (outs),
>> +                   (ins GPR32:$dst, GPR32:$src, brtarget:$BrDst),
>> +                   "if $dst "#OpcodeStr#" $src goto $BrDst",
>> +                   []> {
>> +  bits<4> dst;
>> +  bits<4> src;
>> +  bits<16> BrDst;
>> +
>> +  let Inst{55-52} = src;
>> +  let Inst{51-48} = dst;
>> +  let Inst{47-32} = BrDst;
>> +  let BPFClass = BPF_JMP32;
>> +}
>> +
>> +class JSET_RI_32<string OpcodeStr>
>> +    : TYPE_ALU_JMP<BPF_JSET.Value, BPF_K.Value,
>> +                   (outs),
>> +                   (ins GPR32:$dst, i32imm:$imm, brtarget:$BrDst),
>> +                   "if $dst "#OpcodeStr#" $imm goto $BrDst",
>> +                   []> {
>> +  bits<4> dst;
>> +  bits<16> BrDst;
>> +  bits<32> imm;
>> +
>> +  let Inst{51-48} = dst;
>> +  let Inst{47-32} = BrDst;
>> +  let Inst{31-0} = imm;
>> +  let BPFClass = BPF_JMP32;
>> +}
>> +
>>   multiclass J<BPFJumpOp Opc, string OpcodeStr, PatLeaf Cond, PatLeaf Cond32> {
>>     def _rr : JMP_RR<Opc, OpcodeStr, Cond>;
>>     def _ri : JMP_RI<Opc, OpcodeStr, Cond>;
>> @@ -265,6 +329,10 @@ defm JULT : J<BPF_JLT, "<", BPF_CC_LTU, BPF_CC_LTU_32>;
>>   defm JULE : J<BPF_JLE, "<=", BPF_CC_LEU, BPF_CC_LEU_32>;
>>   defm JSLT : J<BPF_JSLT, "s<", BPF_CC_LT, BPF_CC_LT_32>;
>>   defm JSLE : J<BPF_JSLE, "s<=", BPF_CC_LE, BPF_CC_LE_32>;
>> +def JSET_RR    : JSET_RR<"&">;
>> +def JSET_RI    : JSET_RI<"&">;
>> +def JSET_RR_32 : JSET_RR_32<"&">;
>> +def JSET_RI_32 : JSET_RI_32<"&">;
>>   }
>>
>>   // ALU instructions
>>
>> can solve your inline asm issue. We will discuss whether llvm compiler
>> should be implementing this instruction from source or not.
>
> I'd say 'yes'. clang/llvm should support such asm syntax.
>
> Jose, Eduard,
> Thoughts?

We already support it in GAS:


  $ echo 'if w0 & 0x894b6a55 goto +2' | bpf-unknown-none-as -mdialect=pseudoc -
  $ bpf-unknown-none-objdump -M hex,pseudoc -d a.out
  
  a.out:     file format elf64-bpfle
  
  
  Disassembly of section .text:
  
  0000000000000000 <.text>:
     0:	46 00 02 00 55 6a 4b 89 	if w0&0x894b6a55 goto 0x2


We weren't aware we were diverging with llvm by doing so.  We support
syntax for all the conditional jump instructions using the following
operators:

  BPF_JEQ    ==
  BPF_JGT    >
  BPF_JSGT   s>
  BPF_JGE    >=
  BPF_JSGE   s>=
  BPF_JLT    <
  BPF_JLST   s<
  BPF_JLE    <=
  BPF_JSLE   s<=
  BPF_JSET   &
  BPF_JNE    !=

  reply	other threads:[~2023-11-22 18:38 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-11-21 17:32 max<min after jset Tao Lyu
2023-11-22  0:25 ` Yonghong Song
2023-11-22 14:40   ` [PATCH] C inlined assembly for reproducing max<min Tao Lyu
2023-11-22 18:08     ` Yonghong Song
2023-11-22 18:15       ` Alexei Starovoitov
2023-11-22 18:37         ` Jose E. Marchesi [this message]
2023-11-22 18:51           ` Yonghong Song
2023-11-22 18:39         ` Eduard Zingerman
2023-11-28  4:16 ` max<min after jset Yonghong Song
2023-12-02 10:44   ` [PATCH] C inlined assembly for reproducing max<min Tao Lyu

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=874jhdk51j.fsf@oracle.com \
    --to=jose.marchesi@oracle.com \
    --cc=alexei.starovoitov@gmail.com \
    --cc=andrii@kernel.org \
    --cc=ast@kernel.org \
    --cc=bpf@vger.kernel.org \
    --cc=daniel@iogearbox.net \
    --cc=eddyz87@gmail.com \
    --cc=haoluo@google.com \
    --cc=martin.lau@linux.dev \
    --cc=mathias.payer@nebelwelt.net \
    --cc=meng.xu.cs@uwaterloo.ca \
    --cc=sanidhya.kashyap@epfl.ch \
    --cc=song@kernel.org \
    --cc=tao.lyu@epfl.ch \
    --cc=yonghong.song@linux.dev \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.