[Bug sanitizer/67513] ASAN: Not optimal shadow value check (unlikely condition checked in fast path)
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=67513 Richard Biener changed: What|Removed |Added Target Milestone|9.5 |---
[Bug sanitizer/67513] ASAN: Not optimal shadow value check (unlikely condition checked in fast path)
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=67513 Richard Biener changed: What|Removed |Added Target Milestone|9.4 |9.5 --- Comment #19 from Richard Biener --- GCC 9.4 is being released, retargeting bugs to GCC 9.5.
[Bug sanitizer/67513] ASAN: Not optimal shadow value check (unlikely condition checked in fast path)
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=67513 Jakub Jelinek changed: What|Removed |Added Target Milestone|9.3 |9.4 --- Comment #18 from Jakub Jelinek --- GCC 9.3.0 has been released, adjusting target milestone.
[Bug sanitizer/67513] ASAN: Not optimal shadow value check (unlikely condition checked in fast path)
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=67513 Jakub Jelinek changed: What|Removed |Added Target Milestone|9.2 |9.3 --- Comment #17 from Jakub Jelinek --- GCC 9.2 has been released.
[Bug sanitizer/67513] ASAN: Not optimal shadow value check (unlikely condition checked in fast path)
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=67513 Jakub Jelinek changed: What|Removed |Added Target Milestone|9.0 |9.2 --- Comment #16 from Jakub Jelinek --- GCC 9.1 has been released.
[Bug sanitizer/67513] ASAN: Not optimal shadow value check (unlikely condition checked in fast path)
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=67513 Martin Liška changed: What|Removed |Added Status|ASSIGNED|NEW Assignee|marxin at gcc dot gnu.org |unassigned at gcc dot gnu.org --- Comment #15 from Martin Liška --- Ok, I played with that a bit. It would be possible to optimize the code, but before I see some significant speed up I'm not interested enough.
[Bug sanitizer/67513] ASAN: Not optimal shadow value check (unlikely condition checked in fast path)
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=67513 Martin Liška changed: What|Removed |Added Status|NEW |ASSIGNED CC||marxin at gcc dot gnu.org Assignee|unassigned at gcc dot gnu.org |marxin at gcc dot gnu.org Target Milestone|--- |9.0 --- Comment #14 from Martin Liška --- Let me write the other approach and test it on some program.
[Bug sanitizer/67513] ASAN: Not optimal shadow value check (unlikely condition checked in fast path)
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=67513 Andrew Pinski changed: What|Removed |Added Keywords||missed-optimization Target Milestone|6.5 |---
[Bug sanitizer/67513] ASAN: Not optimal shadow value check (unlikely condition checked in fast path)
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=67513 Richard Biener changed: What|Removed |Added Target Milestone|6.4 |6.5 --- Comment #13 from Richard Biener --- GCC 6.4 is being released, adjusting target milestone.
[Bug sanitizer/67513] ASAN: Not optimal shadow value check (unlikely condition checked in fast path)
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=67513 Jakub Jelinek changed: What|Removed |Added Target Milestone|6.3 |6.4 --- Comment #12 from Jakub Jelinek --- GCC 6.3 is being released, adjusting target milestone.
[Bug sanitizer/67513] ASAN: Not optimal shadow value check (unlikely condition checked in fast path)
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=67513 Richard Biener changed: What|Removed |Added Target Milestone|6.2 |6.3
[Bug sanitizer/67513] ASAN: Not optimal shadow value check (unlikely condition checked in fast path)
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=67513 Richard Biener changed: What|Removed |Added Target Milestone|6.2 |6.3
[Bug sanitizer/67513] ASAN: Not optimal shadow value check (unlikely condition checked in fast path)
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=67513 Richard Biener changed: What|Removed |Added Target Milestone|6.2 |6.3
[Bug sanitizer/67513] ASAN: Not optimal shadow value check (unlikely condition checked in fast path)
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=67513 Richard Biener changed: What|Removed |Added Target Milestone|6.2 |6.3
[Bug sanitizer/67513] ASAN: Not optimal shadow value check (unlikely condition checked in fast path)
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=67513 Jakub Jelinek changed: What|Removed |Added Target Milestone|6.0 |6.2 --- Comment #11 from Jakub Jelinek --- GCC 6.1 has been released.
[Bug sanitizer/67513] ASAN: Not optimal shadow value check (unlikely condition checked in fast path)
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=67513 Marek Polacek changed: What|Removed |Added Status|ASSIGNED|NEW Assignee|mpolacek at gcc dot gnu.org|unassigned at gcc dot gnu.org --- Comment #10 from Marek Polacek --- Not actively working on this.
[Bug sanitizer/67513] ASAN: Not optimal shadow value check (unlikely condition checked in fast path)
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=67513 --- Comment #7 from Yury Gribov --- FYI I'd prefer to keep current BIT_IOR_EXPR approach in asan_expand_check_ifn as it allows for efficient implementation for ARM targets (as compared to two successive branches currently used in LLVM).
[Bug sanitizer/67513] ASAN: Not optimal shadow value check (unlikely condition checked in fast path)
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=67513 --- Comment #8 from Marek Polacek --- (In reply to Yury Gribov from comment #7) > FYI I'd prefer to keep current BIT_IOR_EXPR approach in > asan_expand_check_ifn as it allows for efficient implementation for ARM > targets (as compared to two successive branches currently used in LLVM). Aha. Then I don't know if we want to change this. Please someone decide ;).
[Bug sanitizer/67513] ASAN: Not optimal shadow value check (unlikely condition checked in fast path)
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=67513 Jakub Jelinek changed: What|Removed |Added CC||law at gcc dot gnu.org --- Comment #9 from Jakub Jelinek --- (In reply to Yury Gribov from comment #7) > FYI I'd prefer to keep current BIT_IOR_EXPR approach in > asan_expand_check_ifn as it allows for efficient implementation for ARM > targets (as compared to two successive branches currently used in LLVM). Ugh. Then perhaps we should decide based on some costs. We have LOGICAL_OP_NON_SHORT_CIRCUIT and BRANCH_COST, unfortunately the former is to decide on very cheap computation with roughly the same probabilities, rather than this case where the cheaper condition is much more likely, and BRANCH_COST can't be really compared to insn costs. So perhaps base this say on targetm.have_conditional_execution () ?
[Bug sanitizer/67513] ASAN: Not optimal shadow value check (unlikely condition checked in fast path)
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=67513 Marek Polacek changed: What|Removed |Added Status|UNCONFIRMED |ASSIGNED Last reconfirmed||2015-09-17 CC||mpolacek at gcc dot gnu.org Assignee|unassigned at gcc dot gnu.org |mpolacek at gcc dot gnu.org Target Milestone|--- |6.0 Ever confirmed|0 |1 --- Comment #6 from Marek Polacek --- I'll have a look.
[Bug sanitizer/67513] ASAN: Not optimal shadow value check (unlikely condition checked in fast path)
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=67513 --- Comment #2 from Jakub Jelinek --- Note, the asan pass emits the checks in the order of first testing for non-zero and then doing the sub-quadword comparison (if any), but uses bitwise and in between the two conditions and the choice to expand it this way is the expander and backend choice. To force the behavior you want we'd need to create explicit conditional jump around the second test in the IL.
[Bug sanitizer/67513] ASAN: Not optimal shadow value check (unlikely condition checked in fast path)
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=67513 --- Comment #4 from Andrey Ryabinin --- (In reply to Andrey Ryabinin from comment #3) > (In reply to Yury Gribov from comment #1) > > (In reply to Andrey Ryabinin from comment #0) > > > (shadow value is usually zero). > > > > What makes you think so? AFAIU for less-than-8-byte scalars it's always > > non-zero. > > If 'a' is a pointer to an individual stack/global variable then yes, shadow > will be non-zero. > But if it's part of some array or a struct field, more likely shadow will be > zero. > So I'm not saying that we should blindly switch to non-zero test first, but > it definitely worth trying. > > > I vaguely remember than Kostya did something like this in Clang > > case and it resulted in negligeable improvement. > > clang (version 3.7.0 (tags/RELEASE_370/final)) > tests for non-zero first: > this was a dump from slightly different test-case, here is correct one: foo:# @foo .cfi_startproc # BB#0: pushq %rax .Ltmp0: .cfi_def_cfa_offset 16 movq%rdi, %rax shrq$3, %rax movb2147450880(%rax), %al testb %al, %al jne .LBB0_1 .LBB0_3: movb$0, (%rdi) popq%rax retq .LBB0_1: movl%edi, %ecx andl$7, %ecx movsbl %al, %eax cmpl%eax, %ecx jl .LBB0_3 # BB#2: callq __asan_report_store1 #APP #NO_APP .Lfunc_end0: .size foo, .Lfunc_end0-foo .cfi_endproc
[Bug sanitizer/67513] ASAN: Not optimal shadow value check (unlikely condition checked in fast path)
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=67513 --- Comment #3 from Andrey Ryabinin --- (In reply to Yury Gribov from comment #1) > (In reply to Andrey Ryabinin from comment #0) > > (shadow value is usually zero). > > What makes you think so? AFAIU for less-than-8-byte scalars it's always > non-zero. If 'a' is a pointer to an individual stack/global variable then yes, shadow will be non-zero. But if it's part of some array or a struct field, more likely shadow will be zero. So I'm not saying that we should blindly switch to non-zero test first, but it definitely worth trying. > I vaguely remember than Kostya did something like this in Clang > case and it resulted in negligeable improvement. clang (version 3.7.0 (tags/RELEASE_370/final)) tests for non-zero first: foo:# @foo .cfi_startproc # BB#0: pushq %rax .Ltmp0: .cfi_def_cfa_offset 16 testl %esi, %esi je .LBB0_4 # BB#1: # %.lr.ph movq%rdi, %rax shrq$3, %rax movb2147450880(%rax), %al testb %al, %al jne .LBB0_2 .LBB0_3: movl$0, (%rdi) .LBB0_4: popq%rax retq .LBB0_2: movl%edi, %ecx andl$7, %ecx addl$3, %ecx movsbl %al, %eax cmpl%eax, %ecx jl .LBB0_3 # BB#5: callq __asan_report_store4 #APP #NO_APP .Lfunc_end0: .size foo, .Lfunc_end0-foo .cfi_endproc
[Bug sanitizer/67513] ASAN: Not optimal shadow value check (unlikely condition checked in fast path)
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=67513
--- Comment #5 from Kostya Serebryany ---
(In reply to Yury Gribov from comment #1)
> (In reply to Andrey Ryabinin from comment #0)
> > (shadow value is usually zero).
>
> What makes you think so? AFAIU for less-than-8-byte scalars it's always
> non-zero. I vaguely remember than Kostya did something like this in Clang
> case and it resulted in negligeable improvement.
The code in llvm uses branch weights to indicate that the slow path
is rarely taken.
if (ClAlwaysSlowPath || (TypeSize < 8 * Granularity)) {
// We use branch weights for the slow path check, to indicate that the slow
// path is rarely taken. This seems to be the case for SPEC benchmarks.
TerminatorInst *CheckTerm = SplitBlockAndInsertIfThen(
Cmp, InsertBefore, false, MDBuilder(*C).createBranchWeights(1,
10));
The measurements were done on SPEC 2016 lng ago,
I have no idea whether this is still a good heuristic.
I remember that the difference was not huge, but enough to justify
one extra parameter.
[Bug sanitizer/67513] ASAN: Not optimal shadow value check (unlikely condition checked in fast path)
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=67513 --- Comment #1 from Yury Gribov --- (In reply to Andrey Ryabinin from comment #0) > (shadow value is usually zero). What makes you think so? AFAIU for less-than-8-byte scalars it's always non-zero. I vaguely remember than Kostya did something like this in Clang case and it resulted in negligeable improvement.
