On Thu, Jun 12, 2025 at 12:10 PM Eduard Zingerman <eddyz87@xxxxxxxxx> wrote:
>
> On Thu, 2025-06-12 at 10:19 -0700, Yonghong Song wrote:
> > In one of upstream thread ([1]), there is a discussion about
> > the below inline asm code:
> >
> > if r1 == 0xdeadbeef goto +2;
> > ...
> >
> > In actual llvm backend, the above 0xdeadbeef will actually do
> > sign extension to 64bit value and then compare to register r1.
> >
> > But the code itself does not imply the above semantics. It looks
> > like the comparision is between r1 and 0xdeadbeef. For example,
> > let us at a simple C code:
> > $ cat t1.c
> > int foo(long a) { return a == 0xdeadbeef ? 2 : 3; }
> > $ clang --target=bpf -O2 -c t1.c && llvm-objdump -d t1.o
> > ...
> > w0 = 0x2
> > r2 = 0xdeadbeef ll
> > if r1 == r2 goto +0x1
> > w0 = 0x3
> > exit
> > It does try to compare r1 and 0xdeadbeef.
> >
> > To address the above confusing inline asm issue, llvm backend ([2])
> > added some range checking for such insns and beyond. For the above
> > insn asm, the warning like below
> > warning: immediate out of range, shall fit in int range
> > will be issued. If -Werror is in the compilation flags, the
> > error will be issued.
> >
> > To avoid the above warning/error, the afore-mentioned inline asm
> > should be rewritten to
> >
> > if r1 == -559038737 goto +2;
> > ...
> >
> > Fix a few selftest cases like the above based on insn range checking
> > requirement in [2].
> >
> > [1] https://lore.kernel.org/bpf/70affb12-327b-4882-bd1d-afda8b8c6f56@xxxxxxxxx/
> > [2] https://github.com/llvm/llvm-project/pull/142989
> >
> > Signed-off-by: Yonghong Song <yonghong.song@xxxxxxxxx>
> > ---
>
> Changes like 0xffffffff -> -1 and 0xfffffffe -> -2 look fine,
> but changes like 0xffff1234 -> -60876 are an unnecessary obfuscation,
> maybe we need to reconsider.
I have to agree.
I didn't expect there to be so many warnings.
I thought it would be good to warn for
r3 = 0xdeadbeef
since r3 will have 0xffffFFFFdeadbeef value after assignment,
but warn on
r0 &= 0xFFFF1234
and replacement with -60876 is taking the warning too far.
Also considering Jose's point.
Warning in llvm/gcc on imm32 > UINT_MAX is not correct either.
llvm should probably accept 0xffffFFFFdeadbeef as imm32.
But that is a separate discussion.
