[llvm-dev] 5.0.1-rc2 has been tagged
Tom Stellard via llvm-dev
llvm-dev at lists.llvm.org
Mon Dec 4 06:53:32 PST 2017
On 12/02/2017 10:44 PM, Y Song wrote:
> Hi, Tom,
>
> Considering the severity of this bug, I would like to go ahead to push
> the fix into release_50 branch. The fix has been tested in the trunk and by
> various people as well and I will also make sure all BPF tests passed
> before the push.
>
Hi Yonghong,
Thank you for helping out with LLVM release testing.
Patches need to be approved by the release manager and code owner before they
can be committed to the release branch.
Take a look at http://llvm.org/docs/HowToReleaseLLVM.html#merge-requests
for the process of filling a merge request.
This late in the process we usually only accept critical bugs or regression
fixes.
Can you give me a little more information about this particular bug?
What is the impact to users and how likely are they to encounter this
issue?
Thanks,
Tom
> Thanks!
>
> Yonghong
>
> On Fri, Dec 1, 2017 at 10:18 AM, Y Song <ys114321 at gmail.com> wrote:
>> Hi, Tom,
>>
>> I have a BPF backend bug which is pretty noxious which is introduced
>> in 5.0 and fixed in 6.0 trunk.
>> The detailed of the backport commit is at the end of this email, which
>> is slightly different from
>> 6.0 fix to adjust the asm output format. Is there a way for this bug
>> fix into 5.0.1 release and if yes,
>> what is the process?
>>
>> I have applied the commit below to latest release_50 branch in my
>> local repo and run through all BPF tests
>> and they are fine.
>>
>> Thanks!
>>
>> Yonghong
>>
>> ===== Commit to backport Details =====
>> commit a115edb82180f0c94d692c4abfd631984ada156b
>> Author: Yonghong Song <yhs at fb.com>
>> Date: Mon Oct 16 04:14:53 2017 +0000
>>
>> bpf: fix bug on silently truncating 64-bit immediate
>>
>> We came across an llvm bug when compiling some testcases that 64-bit
>> immediates are silently truncated into 32-bit and then packed into
>> BPF_JMP | BPF_K encoding. This caused comparison with wrong value.
>>
>> This bug looks to be introduced by r308080 (llvm 5.0). The
>> Select_Ri pattern is
>> supposed to be lowered into J*_Ri while the latter only support 32-bit
>> immediate encoding, therefore Select_Ri should have similar immediate
>> predicate check as what J*_Ri are doing.
>>
>> The bug is fixed by
>> git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@315889
>> 91177308-0d34-0410-b5e6-96231b3b80d8
>> in llvm 6.0.
>>
>> This patch is largely the same as the fix in llvm 6.0 except
>> one minor adjustment ("; CHECK: r{{[0-9]+}} = 8589934591 ll" in
>> the original commit
>> to "; CHECK: r{{[0-9]+}} = 8589934591ll" in this patch) for the test case.
>>
>> Reported-by: John Fastabend <john.fastabend at gmail.com>
>> Reported-by: Jakub Kicinski <jakub.kicinski at netronome.com>
>> Signed-off-by: Jiong Wang <jiong.wang at netronome.com>
>> Reviewed-by: Yonghong Song <yhs at fb.com>
>>
>> diff --git a/lib/Target/BPF/BPFISelLowering.cpp
>> b/lib/Target/BPF/BPFISelLowering.cpp
>> index 81b0aa7..5740b49 100644
>> --- a/lib/Target/BPF/BPFISelLowering.cpp
>> +++ b/lib/Target/BPF/BPFISelLowering.cpp
>> @@ -578,11 +578,15 @@
>> BPFTargetLowering::EmitInstrWithCustomInserter(MachineInstr &MI,
>> .addReg(LHS)
>> .addReg(MI.getOperand(2).getReg())
>> .addMBB(Copy1MBB);
>> - else
>> + else {
>> + int64_t imm32 = MI.getOperand(2).getImm();
>> + // sanity check before we build J*_ri instruction.
>> + assert (isInt<32>(imm32));
>> BuildMI(BB, DL, TII.get(NewCC))
>> .addReg(LHS)
>> - .addImm(MI.getOperand(2).getImm())
>> + .addImm(imm32)
>> .addMBB(Copy1MBB);
>> + }
>>
>> // Copy0MBB:
>> // %FalseValue = ...
>> diff --git a/lib/Target/BPF/BPFInstrInfo.td b/lib/Target/BPF/BPFInstrInfo.td
>> index f683578..56f0f9c 100644
>> --- a/lib/Target/BPF/BPFInstrInfo.td
>> +++ b/lib/Target/BPF/BPFInstrInfo.td
>> @@ -464,7 +464,7 @@ let usesCustomInserter = 1 in {
>> (ins GPR:$lhs, i64imm:$rhs, i64imm:$imm,
>> GPR:$src, GPR:$src2),
>> "# Select PSEUDO $dst = $lhs $imm $rhs ? $src : $src2",
>> [(set i64:$dst,
>> - (BPFselectcc i64:$lhs, (i64 imm:$rhs), (i64
>> imm:$imm), i64:$src, i64:$src2))]>;
>> + (BPFselectcc i64:$lhs, (i64immSExt32:$rhs),
>> (i64 imm:$imm), i64:$src, i64:$src2))]>;
>> }
>>
>> // load 64-bit global addr into register
>> diff --git a/test/CodeGen/BPF/select_ri.ll b/test/CodeGen/BPF/select_ri.ll
>> index c4ac376..3610d40 100644
>> --- a/test/CodeGen/BPF/select_ri.ll
>> +++ b/test/CodeGen/BPF/select_ri.ll
>> @@ -25,3 +25,38 @@ entry:
>> }
>>
>> attributes #0 = { norecurse nounwind readonly }
>> +
>> +; test immediate out of 32-bit range
>> +; Source file:
>> +
>> +; unsigned long long
>> +; load_word(void *buf, unsigned long long off)
>> +; asm("llvm.bpf.load.word");
>> +;
>> +; int
>> +; foo(void *buf)
>> +; {
>> +; unsigned long long sum = 0;
>> +;
>> +; sum += load_word(buf, 100);
>> +; sum += load_word(buf, 104);
>> +;
>> +; if (sum != 0x1ffffffffULL)
>> +; return ~0U;
>> +;
>> +; return 0;
>> +;}
>> +
>> +; Function Attrs: nounwind readonly
>> +define i32 @foo(i8*) local_unnamed_addr #0 {
>> + %2 = tail call i64 @llvm.bpf.load.word(i8* %0, i64 100)
>> + %3 = tail call i64 @llvm.bpf.load.word(i8* %0, i64 104)
>> + %4 = add i64 %3, %2
>> + %5 = icmp ne i64 %4, 8589934591
>> +; CHECK: r{{[0-9]+}} = 8589934591ll
>> + %6 = sext i1 %5 to i32
>> + ret i32 %6
>> +}
>> +
>> +; Function Attrs: nounwind readonly
>> +declare i64 @llvm.bpf.load.word(i8*, i64) #1
>>
>> On Wed, Nov 29, 2017 at 4:19 PM, Tom Stellard via llvm-dev
>> <llvm-dev at lists.llvm.org> wrote:
>>> Hi,
>>>
>>> I've tagged the 5.0.1-rc2 release, go ahead and start testing and report
>>> your results.
>>>
>>> -Tom
>>> _______________________________________________
>>> LLVM Developers mailing list
>>> llvm-dev at lists.llvm.org
>>> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev
More information about the llvm-dev
mailing list