On Thu, Oct 1, 2026 at 1:47 PM Uros Bizjak <[email protected]> wrote: > > On Thu, Oct 1, 2026 at 1:10 PM Richard Biener > <[email protected]> wrote: > > > > On Thu, Oct 1, 2026 at 9:13 AM Uros Bizjak <[email protected]> wrote: > > > > > > On Wed, Sep 30, 2026 at 2:23 PM Richard Biener > > > <[email protected]> wrote: > > > > > > > > On Mon, Sep 28, 2026 at 11:44 AM Uros Bizjak <[email protected]> wrote: > > > > > > > > > > On Mon, Sep 28, 2026 at 9:39 AM Richard Biener > > > > > <[email protected]> wrote: > > > > > > > > > > > > On Sun, Sep 27, 2026 at 9:11 PM Uros Bizjak <[email protected]> > > > > > > wrote: > > > > > > > > > > > > > > For an output operand whose constraint allows both a register and > > > > > > > memory (e.g. "=rm"), expand_asm_stmt only binds the operand > > > > > > > directly > > > > > > > to the address of its underlying storage when the operand's value > > > > > > > is > > > > > > > a bare, non-ADDR_EXPR-based MEM_REF or a plain, memory-resident > > > > > > > DECL. > > > > > > > Every other lvalue -- most commonly a struct/union member, an > > > > > > > array > > > > > > > element, or a complex number's real/imaginary part -- is instead > > > > > > > always routed through a fresh pseudo register that gets copied to > > > > > > > the > > > > > > > real destination in a separate insn after the asm, even though > > > > > > > nothing about the constraint requires it. > > > > > > > > > > > > > > Extend the set of output values eligible for direct addressing to > > > > > > > also cover COMPONENT_REF (except through a bit-field, which is not > > > > > > > byte-addressable and still needs the existing register-based > > > > > > > read-modify-write handling), ARRAY_REF, ARRAY_RANGE_REF, > > > > > > > REALPART_EXPR > > > > > > > and IMAGPART_EXPR. > > > > > > > > > > > > At least the comment needs updating, but I also wonder why we need > > > > > > the else {} > > > > > > and whether we can more easily put a condition on that with the if > > > > > > case being > > > > > > seemingly the more "default" handling? > > > > > > > > > > > > Also the DECL_P handling would need to apply to the MEM_REF && > > > > > > ADDR_EXPR > > > > > > case (not sure what the DECL_P handling is about - this code really > > > > > > lacks comments...). > > > > > > And I'd like to see using REFERENCE_CLASS_P (val) && (... > > > > > > exclusions ...) rather > > > > > > than enumerating parts of it that are OK. > > > > > > > > > > I based the whole idea on your 2022 patch [1], where the patch > > > > > deliberately left the other tcc_reference operands unexplored. Since > > > > > my GIMPLE knowledge is limited, my patch just trivially adds some of > > > > > them (with the help of AI) to the existing condition (the ones that I > > > > > can easily check with the C examples). > > > > > > > > I probably had the same questions back then and decided to simply fixup > > > > the > > > > INDIRECT_REF case. Maybe AI can help deciphering the current code. > > > > I'd probably start testing by doing just > > > > > > > > if (allows_mem > > > > || ! allows_reg > > > > ... > > > > > > > > but I do question what we have the else {} case for (is there any test > > > > coverage? > > > > a comment should probably point to it!) > > > > > > Claude was more than happy to jump to the above task: > > > > > > --q-- > > > The else branch isn't just a fallback — for one real case it's already > > > doing the right thing, by accident of mechanism. > > > > > > void use(int); > > > void foo (void) > > > { > > > int x; > > > asm ("mov %%fs, %0" : "=rm" (x)); > > > use(x); > > > } > > > > > > compiles today to: > > > > > > mov %fs, %edi > > > jmp use@PLT > > > > > > No store, no reload — the value stays in a register and gets reused > > > directly. That's the good outcome we were worried about losing > > > throughout this whole thread. And it's happening through the else > > > branch: x is a local, non-addressable scalar, so by the time this > > > reaches cfgexpand.cc its def is an SSA_NAME, not a DECL_P. None of the > > > direct-path disjuncts fire for an SSA_NAME, so it falls into else, > > > gets a temp pseudo via assign_temp, and expand_assignment against an > > > SSA_NAME target doesn't do a real store — it just establishes that > > > pseudo as the SSA partition's value. The TREE_CODE (val) == SSA_NAME > > > special case right there (set_reg_attrs_for_decl_rtl) is attaching > > > proper debug/alias attributes to that temp before it's wired in as the > > > partition's register. > > > --/q-- > > > > Ah, I was probably confused by assign_temp followed by validize_mem. > > But expand_expr of 'x' above would, given allows_reg and thus EXPAND_WRITE > > also expand to a pseudo, but in "cheaper" ways. Verified by adding 1 || to > > the > > if () condition. > > > > So we're looking for a testcase where the if () part doesn't work (ICE > > or wrong-code) > > You will definitely need to reject > > DECL_BIT_FIELD (TREE_OPERAND (val, 1)) > > because the variable isn't byte addressable, please see pr124209-5.c > in my original submission.
That might be a bug in expand_expr with EXPAND_WRITE though? may_be_nonaddressable_p indicates any reverse storage order access might be problematic as well as hard-registers For the record, testsuite fallout is +FAIL: g++.dg/ext/pr106877.C -std=gnu++20 (test for excess errors) +FAIL: g++.dg/ext/pr106877.C -std=gnu++29 (test for excess errors) +FAIL: g++.dg/ext/pr106877.C -std=gnu++98 (test for excess errors) +FAIL: g++.dg/ext/pr84683.C -std=gnu++20 (test for excess errors) +FAIL: g++.dg/ext/pr84683.C -std=gnu++29 (test for excess errors) +FAIL: g++.dg/ext/pr84683.C -std=gnu++98 (test for excess errors) +FAIL: g++.dg/ext/pr84828.C -std=gnu++20 (test for excess errors) +FAIL: g++.dg/ext/pr84828.C -std=gnu++29 (test for excess errors) +FAIL: g++.dg/ext/pr84828.C -std=gnu++98 (test for excess errors) +FAIL: gcc.c-torture/compile/asmgoto-4.c -O0 scan-rtl-dump-times reload "Inserting insn reload after in bb" 2 +FAIL: gcc.c-torture/compile/asmgoto-4.c -O1 scan-rtl-dump-times reload "Inserting insn reload after in bb" 2 +FAIL: gcc.c-torture/compile/asmgoto-4.c -O2 scan-rtl-dump-times reload "Inserting insn reload after in bb" 2 +FAIL: gcc.c-torture/compile/asmgoto-4.c -O2 -flto -fno-use-linker-plugin -flto-partition=none scan-rtl-dump-times reload "Inserting insn reload after in bb" 2 +FAIL: gcc.c-torture/compile/asmgoto-4.c -O2 -flto -fuse-linker-plugin -fno-fat-lto-objects scan-rtl-dump-times reload "Inserting insn reload after in bb" 2 +FAIL: gcc.c-torture/compile/asmgoto-4.c -O3 -g scan-rtl-dump-times reload "Inserting insn reload after in bb" 2 +FAIL: gcc.c-torture/compile/asmgoto-4.c -Os scan-rtl-dump-times reload "Inserting insn reload after in bb" 2 Richard. > > Uros.
