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. Uros.
