Am 28.07.23 um 07:04 schrieb senthilkumar.selva...@microchip.com:
On Thu, 2023-07-27 at 15:11 +0200, Georg-Johann Lay wrote:
EXTERNAL EMAIL: Do not click links or open attachments unless you know the
content is safe
Am 17.07.23 um 13:33 schrieb SenthilKumar.Selvaraj--- via Gcc:
Hi,
The avr target has a bunch of patterns that directly set hard regs at
expand time, like so
The correct approach would be to use usual predicates together with
constraints that describe the register instead of hard regs, e.g.
(match_operand:HI n "register_operand" "R18_2") for a 2-byte register
that starts at R18 instead of (reg:HI 18). I deprecated and removed
constraints starting with "R" long ago in order to get "R" free for that
purpose.
Some years ago I tried such constraints (and hence also zoo of new
register classes that are required to accommodate them). The resulting
code quality was so bad that I quickly abandoned that approach, and IIRC
there were also spill fails. Appears that reload / ira was overwhelmed
by the multitude of new reg classes and took sub-optimal decisions.
The way out was more of explicit hard regs in expand, together with
awkward functionalities like avr_fix_operands (PR63633) and the
functions that use it. That way we get correct code without performance
penalties in unrelated places.
Most of such insns are explicitly modelling hand-written asm functions
in libgcc, because most of these functions have a footprint smaller than
the default ABI. And some functions have an interface not complying to
default ABI.
For the case of cpymem etc from below, explicit hard registers were used
because register allocator did a bad job when using constraints like "e"
(X, Y, or Z).
I guessed that much. Yes, using constraints works - I used "x" and "z" that
directly correspond to REG_X and REG_Z (ignore the weird operand numbering).
diff --git a/gcc/config/avr/avr.md b/gcc/config/avr/avr.md
index be0f8dcbe0e..6c6c4e4e212 100644
--- a/gcc/config/avr/avr.md
+++ b/gcc/config/avr/avr.md
@@ -1148,20 +1148,20 @@
;; "cpymem_qi"
;; "cpymem_hi"
(define_insn_and_split "cpymem_<mode>"
- [(set (mem:BLK (reg:HI REG_X))
- (mem:BLK (reg:HI REG_Z)))
+ [(set (mem:BLK (match_operand:HI 3 "register_operand" "+x"))
+ (mem:BLK (match_operand:HI 4 "register_operand" "+z")))
(unspec [(match_operand:QI 0 "const_int_operand" "n")]
UNSPEC_CPYMEM)
(use (match_operand:QIHI 1 "register_operand" "<CPYMEM_r_d>"))
- (clobber (reg:HI REG_X))
- (clobber (reg:HI REG_Z))
+ (clobber (match_dup 3))
+ (clobber (match_dup 4))
With some other insns I encountered problem with clobber (match_dup),
similar to PR50788, and thus used clobber of operand with constraint as
proposed in that PR. See also comments for insn "*add.for.eqne.<mode>".
(clobber (reg:QI LPM_REGNO))
(clobber (match_operand:QIHI 2 "register_operand" "=1"))]
""
"#"
"&& reload_completed"
- [(parallel [(set (mem:BLK (reg:HI REG_X))
- (mem:BLK (reg:HI REG_Z)))
+ [(parallel [(set (mem:BLK (match_dup 3))
+ (mem:BLK (match_dup 4)))
This split happens when reload_completed, thus using hard regs should be
okay (match_dup will be X or Z anyway).
(unspec [(match_dup 0)]
UNSPEC_CPYMEM)
(use (match_dup 1))
I know you did these changes a long time ago, but do you happen to have any
test cases lying around that I can use to see if LRA does a better job than
classic reload?
No. But the test cases I used were straight forward lines of code.
Johann
Vladimir, given that classic reload handled such hardcoded hard regs just
fine, should LRA also be able to deal with them the same way? Or is this
something that LRA is not going to support?
Regards
Senthil
