On Mon, Sep 4, 2023 at 4:57 PM Uros Bizjak <ubiz...@gmail.com> wrote: > > On Mon, Sep 4, 2023 at 2:28 AM Hongtao Liu <crazy...@gmail.com> wrote: > > > > > > > > > I think there should be some constraint which explicitly has > > > > > > > > all the 32 > > > > > > > > GPRs, like there is one for just all 16 GPRs (h), so that > > > > > > > > regardless of > > > > > > > > -mapx-inline-asm-use-gpr32 one can be explicit what the inline > > > > > > > > asm wants. > > > > > > > > > > > > > > > > Also, what about the "g" constraint? Shouldn't there be > > > > > > > > another for "g" > > > > > > > > without r16..r31? What about the various other memory > > > > > > > > constraints ("<", "o", ...)? > > > > > > > > > > > > > > I think we should leave all existing constraints as they are, so > > > > > > > "r" > > > > > > > covers only GPR16, "m" and "o" to only use GPR16. We can then > > > > > > > introduce "h" to instructions that have the ability to handle > > > > > > > EGPR. > > > > > > > This would be somehow similar to the SSE -> AVX512F transition, > > > > > > > where > > > > > > > we still have "x" for SSE16 and "v" was introduced as a separate > > > > > > > register class for EVEX SSE registers. This way, asm will be > > > > > > > compatible, when "r", "m", "o" and "g" are used. The new memory > > > > > > > constraint "Bt", should allow new registers, and should be added > > > > > > > to > > > > > > > the constraint string as a separate constraint, and conditionally > > > > > > > enabled by relevant "isa" (AKA "enabled") attribute. > > > > > > > > > > > > The extended constraint can work for registers, but for memory it > > > > > > is more > > > > > > complicated. > > > > > > > > > > Yes, unfortunately. The compiler assumes that an unchangeable register > > > > > class is used for BASE/INDEX registers. I have hit this limitation > > > > > when trying to implement memory support for instructions involving > > > > > 8-bit high registers (%ah, %bh, %ch, %dh), which do not support REX > > > > > registers, also inside memory operand. (You can see the "hack" in e.g. > > > > > *extzvqi_mem_rex64" and corresponding peephole2 with the original > > > > > *extzvqi pattern). I am aware that dynamic insn-dependent BASE/INDEX > > > > > register class is the major limitation in the compiler, so perhaps the > > > > > strategy on how to override this limitation should be discussed with > > > > > the register allocator author first. Perhaps adding an insn attribute > > > > > to insn RTX pattern to specify different BASE/INDEX register sets can > > > > > be a better solution than passing insn RTX to the register allocator. > > > > > > > > > > The above idea still does not solve the asm problem on how to select > > > > > correct BASE/INDEX register set for memory operands. > > > > The current approach disables gpr32 for memory operand in asm_operand > > > > by default. but can be turned on by options > > > > ix86_apx_inline_asm_use_gpr32(users need to guarantee the instruction > > > > supports gpr32). > > > > Only ~ 5% of total instructions don't support gpr32, reversed approach > > > > only gonna get more complicated. > > > > > > I'm not referring to the reversed approach, just want to point out > > > that the same approach as you proposed w.r.t. to memory operand can be > > > achieved using some named insn attribute that would affect BASE/INDEX > > > register class selection. The attribute could default to gpr32 with > > > APX, unless the insn specific attribute has e.g. nogpr32 value. See > > > for example how "enabled" and "preferred_for_*" attributes are used. > > > Perhaps this new attribute can also be applied to separate > > > alternatives. > > Yes, for xop/fma4/3dnow instructions, I think we can use isa attr like > > (define_attr "gpr32" "0, 1" > > (cond [(eq_attr "isa" "fma4") > > (const_string "0")] > > (const_string "1"))) > > Just a nit, can the member be named "map0" and "map1"? The code will > then look like: > > if (get_attr_gpr32 (insn) == GPR32_MAP0) ... > > instead of: > > if (get_attr_gpr32 (insn) == GPR32_0) ... > > > But still, we need to adjust memory constraints in the pattern. > > I guess the gpr32 property is the same for all alternatives of the > insn pattern. In this case, "m" "g" and "a" constraints could remain > as they are, the final register class will be adjusted (by some target > hook?) based on the value of gpr32 attribute. I'm worried that not all rtl optimizers after post_reload will respect base/index_reg_class regarding the insn they belong to. if they just check if it's a legitimate memory/address (the current legitimate_address doesn't have a corresponding insn to pass down), m/g/a will still generate invalid instruction. So a defensive programming is to explicitly modifying the constraint. > > > Ideally, gcc includes encoding information for every instruction, > > (.i.e. map0/map1), so that we can determine the attribute value of > > gpr32 directly from this information. > > I think the right tool for this is attribute infrastructure of insn > patterns. We can set the default, set precise value of the insns, or > calculate attribute from some other attribute in a quite flexible way. > Other than that, adjusting BASE/INDEX register class of the RA pass is > the infrastructure change, but perhaps similar to the one you > proposed. Yes. > > Uros.
-- BR, Hongtao