[PATCH] D130956: [X86][MC] Always emit `rep` prefix for `bsf`
Craig Topper via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Tue Aug 2 20:01:43 PDT 2022
craig.topper added inline comments.
================
Comment at: llvm/test/CodeGen/X86/peephole-na-phys-copy-folding.ll:376
%cmp = icmp sgt i32 %val, 0
%res = tail call i32 asm "bsfl $1,$0", "=r,r,~{cc},~{dirflag},~{fpsr},~{flags}"(i32 %val)
store i32 %res, ptr %mem, align 4
----------------
skan wrote:
> craig.topper wrote:
> > skan wrote:
> > > aaronpuchert wrote:
> > > > pengfei wrote:
> > > > > aaronpuchert wrote:
> > > > > > Goes a bit too far I think. We can turn a generic builtin into `rep; bsf`, but if the inline assembly explicitly asks for `bsf` I think we should emit that.
> > > > > You are right. So this is to check *no* REP prefix was generated. :)
> > > > Sorry, I missed the `-NOT`. But is that needed? After all the `-NEXT` shouldn't match if there is a `rep` in between.
> > > >
> > > > If there is a reason to keep this, the second occurrence should likely be `CHECK64` instead of `CHECK32`.
> > > I think the two `CHECK-NOT` are redundant here b/c `CHECK-NEXT` can gurantee `#APP` is followed by `bsfl`.
> > Doesn't the CHECK-NEXT only guarantee that the next line contains bsfl. It doesn't rule out any text before the bsfl.
> I think if there is `rep` between `#APP` and `#NO_APP`, the following check will fail.
> ```
> ; CHECK32-NEXT: #APP
> ; CHECK32-NEXT: bsfl %edx, %edx
> ; CHECK32-NEXT: #NO_APP
> ```
The test still passes with this change
```
diff --git a/llvm/test/CodeGen/X86/peephole-na-phys-copy-folding.ll b/llvm/test/CodeGen/X86/peephole-na-phys-copy-folding.ll
index f3d4b6221d08..4c7094d5c5f0 100644
--- a/llvm/test/CodeGen/X86/peephole-na-phys-copy-folding.ll
+++ b/llvm/test/CodeGen/X86/peephole-na-phys-copy-folding.ll
@@ -371,7 +371,7 @@ define i1 @asm_clobbering_flags(ptr %mem) nounwind {
entry:
%val = load i32, ptr %mem, align 4
%cmp = icmp sgt i32 %val, 0
- %res = tail call i32 asm "bsfl $1,$0", "=r,r,~{cc},~{dirflag},~{fpsr},~{flags}"(i32 %val)
+ %res = tail call i32 asm "rep bsfl $1,$0", "=r,r,~{cc},~{dirflag},~{fpsr},~{flags}"(i32 %val)
store i32 %res, ptr %mem, align 4
ret i1 %cmp
```
That puts rep on the same line before bsfl in the output. Every test change in this review shows rep on the same line as the bsfl.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D130956/new/
https://reviews.llvm.org/D130956
More information about the llvm-commits
mailing list