yonghong-song added a comment.
@pchaigno @efriedma I added an IR pass in BPF backend to remove these
stacksave/stackrestore intrinsics. https://reviews.llvm.org/D111897
@pchaigno Could you test with https://reviews.llvm.org/D111897 instead?
Repository:
rG LLVM Github Monorepo
CHANGES SINCE
yonghong-song added a comment.
@pchaigno as @efriedma suggested, we can add some transformation in BPF target
IR passes to ignore @llvm.stacksave() and @llvm.stackrestore(). I should have a
patch ready soon.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
efriedma added a comment.
I don't think my opinion has changed here. I'm against the solution proposed
in this patch. The other solutions discussed in the review seem fine. (The
simplest is just to make the bpf backend ignore stacksave/stackrestore calls.)
Repository:
rG LLVM Github
pchaigno added a comment.
@efriedma @rsmith Did you get a chance to take a look? This regression is still
affecting LLVM, including the recently released v13.0.0.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D107882/new/
yonghong-song added a comment.
I checked EmitAutoVarAlloca(). It emits the llvm.stacksave() due to
// If the type is variably-modified, emit all the VLA sizes for it.
if (Ty->isVariablyModifiedType())
EmitVariablyModifiedType(Ty);
Here, in order not to generate llvm.stacksave(), we need
yonghong-song added a comment.
@efriedma Thanks for suggestion! Let me look at
CodeGenFunction::EmitAutoVarAlloca() call instead.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D107882/new/
https://reviews.llvm.org/D107882
efriedma added a comment.
I'd prefer not to mess with the AST if we don't need to; more differences
between targets make it harder to understand any issues that come up.
BPF-flavored C already has enough weird differences without adding unnecessary
changes.
If all you need is to avoid
yonghong-song added a comment.
@rsmith ping again, did you get a time to take a look?
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D107882/new/
https://reviews.llvm.org/D107882
___
cfe-commits mailing
yonghong-song added a comment.
@rsmith ping again. Could you help take a look at this patch? Thanks!
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D107882/new/
https://reviews.llvm.org/D107882
___
yonghong-song added a comment.
ping. @rsmith could you help take a look at the patch? Thanks!
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D107882/new/
https://reviews.llvm.org/D107882
___
cfe-commits
pchaigno added a comment.
I tested this patch by running it through Cilium's CI which compiles, loads,
and validates the behavior of hundreds of slightly different BPF programs. The
tests cover several different kernels and all three BPF instruction set
extensions (mcpu={v1,v2,v3}). It did not
yonghong-song updated this revision to Diff 365893.
yonghong-song added a comment.
- fix clant-format warnings.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D107882/new/
https://reviews.llvm.org/D107882
Files:
clang/lib/Sema/SemaType.cpp
yonghong-song created this revision.
yonghong-song added a reviewer: rsmith.
Herald added a subscriber: dexonsmith.
yonghong-song requested review of this revision.
Herald added projects: clang, LLVM.
Herald added subscribers: llvm-commits, cfe-commits.
Paul Chaignon reported a bpf verifier
13 matches
Mail list logo
