On Sat, 2020-06-27 at 13:38 +0200, Marc Glisse wrote: > On Sat, 27 Jun 2020, Ilya Leoshkevich via Gcc-patches wrote: > > > Is there something specific that a compiler user should look out > > for? > > For example, here is the kernel code, from which the test was > > derived: > > > > static inline void atomic_add(int i, atomic_t *v) > > { > > #ifdef CONFIG_HAVE_MARCH_Z196_FEATURES > > if (__builtin_constant_p(i) && (i > -129) && (i < 128)) { > > __atomic_add_const(i, &v->counter); > > return; > > } > > #endif > > __atomic_add(i, &v->counter); > > } > > > > It looks very straightforward - can there still be something wrong > > with its usage of b_c_p? > > > > > I'd recommend looking at the .ssa dump and walk forward from > > > there if > > > the .ssa dump looks correct. > > > > Well, 021t.ssa already has: > > > > __attribute__((gnu_inline)) > > __atomic_add_const (intD.6 valD.2269, intD.6 * ptrD.2270) > > { > > intD.6 val_3(D) = valD.2269; > > intD.6 * ptr_2(D) = ptrD.2270; > > ;; basic block 2, loop depth 0, maybe hot > > ;; prev block 0, next block 1, flags: (NEW) > > ;; pred: ENTRY (FALLTHRU) > > # .MEM_4 = VDEF <.MEM_1(D)> > > __asm__ __volatile__("asi %0,%1 > > " : "ptr" "=Q" *ptr_2(D) : "val" "i" val_3(D), "Q" *ptr_2(D) : > > "memory", "cc"); > > # VUSE <.MEM_4> > > return; > > ;; succ: EXIT > > > > } > > > > which is, strictly speaking, not correct, because val_3(D) and > > valD.2269 are not constant. But as far as I understand we are > > willing > > to tolerate trees like this until a certain point. > > > > What is this point supposed to be? If I understood you right, > > 106t.thread1 is already too late - why is it so? > > Small remark: shouldn't __atomic_add_const be marked with the > always_inline attribute, since it isn't usable when it isn't inlined?
I agree, this would be a good improvement (from both readability and correctness perspectives). Still, I just tried it, and unfortunately it did not help with the issue at hand, most likely because the function is inlined either way. 021t.ssa still contains: __attribute__((always_inline, gnu_inline)) __atomic_add_const (intD.6 valD.2269, intD.6 * ptrD.2270) { and threading still eliminates its inlined version.