On Wed, Jun 3, 2026 at 2:50 PM Jeffrey Law <[email protected]> wrote:
>
>
>
> On 2/3/2026 9:04 PM, Sebastian Huber wrote:
> > ----- Am 4. Feb 2026 um 4:38 schrieb Sebastian Huber
> > [email protected]:
> >
> >> Hello Jeff,
> >>
> >> ----- Am 29. Dez 2025 um 20:08 schrieb Jeffrey Law
> >> [email protected]:
> >>
> >>> On 12/28/2025 7:03 PM, Sebastian Huber wrote:
> >>>> I used this test case to double check that the shifting is now correct:
> >>>>
> >>>> int a(void);
> >>>> int b(void);
> >>>> int c(int);
> >>>> int f(int *i)
> >>>> {
> >>>> if (c(i[0]) || c(i[1]) || c(i[2]) || c(i[3]) || c(i[4]) ||
> >>>> c(i[5]) || c(i[6]) || c(i[7]) || c(i[8]) || c(i[9]) ||
> >>>> c(i[10]) || c(i[11]) || c(i[12]) || c(i[13]) || c(i[14]) ||
> >>>> c(i[15]) || c(i[16]) || c(i[17]) || c(i[18]) || c(i[19]) ||
> >>>> c(i[20]) || c(i[21]) || c(i[22]) || c(i[23]) || c(i[24]) ||
> >>>> c(i[25]) || c(i[26]) || c(i[27]) || c(i[28]) || c(i[29]) ||
> >>>> c(i[30]) || c(i[31]) || c(i[32]) || c(i[33]) || c(i[34]) ||
> >>>> c(i[35]) || c(i[36]) || c(i[37]) || c(i[38]) || c(i[39])) {
> >>>> return a();
> >>>> } else {
> >>>> return b();
> >>>> }
> >>>> }
> >>>>
> >>>> Interestingly, GCC now reuses the "amoor.w zero,zero" operations (see "j
> >>>> .L46").
> >>> Right. That's not a huge surprise to me. If we look at the gimple we
> >>> see:
> >>>
> >>>
> >>>> ;; basic block 3, loop depth 0
> >>>> ;; pred: 2
> >>>> __atomic_fetch_or_4 (&__gcov8.f[0], 1, 0);
> >>>> __atomic_fetch_or_4 (&MEM <long long int> [(void *)&__gcov8.f + 4B],
> >>>> 0, 0);
> >>>> __atomic_fetch_or_4 (&__gcov8.f[1], 0, 0);
> >>>> __atomic_fetch_or_4 (&MEM <long long int> [(void *)&__gcov8.f +
> >>>> 12B], 0, 0);
> >>>> _8 = a (i_3(D)); [tail call]
> >>>> goto <bb 5>; [100.00%]
> >>>> ;; succ: 5
> >>>>
> >>>> ;; basic block 4, loop depth 0
> >>>> ;; pred: 2
> >>>> __atomic_fetch_or_4 (&__gcov8.f[0], 0, 0);
> >>>> __atomic_fetch_or_4 (&MEM <long long int> [(void *)&__gcov8.f + 4B],
> >>>> 0, 0);
> >>>> __atomic_fetch_or_4 (&__gcov8.f[1], 1, 0);
> >>>> __atomic_fetch_or_4 (&MEM <long long int> [(void *)&__gcov8.f +
> >>>> 12B], 0, 0);
> >>>> _6 = b (0); [tail call]
> >>> So to improve the code you need to recognize the atomic_fetch_or_4 where
> >>> the object is IOR'd with the constant 0 as a nop and remove those
> >>> statements (or not emit them to begin with). In general our optimizers
> >>> don't do a whole lot with atomics right now.
> >> should I try to add these optimizations for a constant 0 to the patch or
> >> can we
> >> do this in a follow up patch?
> >>
> >> Is there a simple check to figure out if "tree counter" is a constant 0?
> > What doesn't work is this:
> [ ... ]
> So I was reviewing a patch from another team here in Qualcomm that was
> meant to optimize a NOP RMW operation and concluding that in general
> it's never OK to elide the "W" step of an RMW atomic, even relaxed atomics.
>
> That comes into play here. Essentially the gimple optimizers and such
> have to leave those RMW operations alone, thus any optimization has to
> happen during generation of the profiling code.
>
> Let's look at the code the profiler generates:
>
> > ;; basic block 7, loop depth 0, count 365072224 (estimated locally,
> > freq 0.3400), maybe hot
> > ;; prev block 6, next block 8, flags: (NEW)
> > ;; pred: 6 [34.0% (guessed)] count:365072224 (estimated
> > locally, freq 0.3400) (TRUE_VALUE,EXECUTABLE)
> > _130 = 0 | 1;
> > _249 = ~0;
> > _250 = _130 & _249;
> > _251 = 0 & _249;
> > __atomic_fetch_or_8 (&__gcov8.f[0], _250, 0);
> > __atomic_fetch_or_8 (&__gcov8.f[1], _251, 0);
> > __atomic_add_fetch_8 (&__gcov0.f[1], 1, 0);
> > goto <bb 86>; [100.00%]
>
> This explains why your hack didn't work. Let's focus on that second
> atomic_fetch_or_8. That statement references _251, not the integer
> constant zero. So your integer_zerop check naturally failed.
>
> As noted earlier, once we've got this code, we can't eliminate the
> fetch_or, even though it looks like a NOP. It must be optimized before
> generating code.
>
> At least some of these are coming from this code in instrument_decisions:
>
> > 1168 /* _true &= ~mask, _false &= ~mask */
> > 1169 counters next;
> > 1170 next[2] = emit_bitwise_op (e, prev[2], BIT_NOT_EXPR);
> > 1171 next[0] = emit_bitwise_op (e, prev[0], BIT_AND_EXPR,
> > next[2]);
> > 1172 next[1] = emit_bitwise_op (e, prev[1], BIT_AND_EXPR,
> > next[2]);
>
> If we look at prev[1] we'd see it's:
>
>
> > <integer_cst 0x7ffff70496f0 type <integer_type 0x7ffff7247738 long
> > int> constant 0>
>
> Which would return true for integer_zerop. That allows you to conclude
> the BIT_AND produces the value zero which we then pass into the atomic
> IOR in the loop immediately below. You just need to track that state
> and avoid generating the atomic IOR in that case.
>
> I haven't really dove into the code, but it seems like you could
> probably try to fold the operation to a constant in emit_bitwise_op and
> return the folded constant rather than an SSA_NAME. Then you elide the
> IOR when next[k] is an integer_zerop.
>
> Does that make sense to you?
So I looked into the code right now emit_bitwise_op does:
```
/* Emit LHS = OP1 <OP> OP2 on edges. */
tree
emit_bitwise_op (edge e, tree op1, tree_code op, tree op2 = NULL_TREE)
{
tree lhs = make_ssa_name (gcov_type_node);
gassign *w = gimple_build_assign (lhs, op, op1, op2);
gsi_insert_on_edge (e, w);
return lhs;
}
```
If instead that changes to use gimple_build instead of
gimple_build_assign, it would just work.
Also it seems like we could build up a sequence and then insert that
sequence on the edge instead of inserting on the edge at each build
assignment.
Note I would do the change to use gimple_build separately from this
optimization.
Thanks,
Andrea
>
> jeff
>
>
>
>
>
>
>
