From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mo4-p00-ob.smtp.rzone.de (mo4-p00-ob.smtp.rzone.de [81.169.146.216]) by sourceware.org (Postfix) with ESMTPS id E4CB038555AE for ; Thu, 27 Jul 2023 13:11:26 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org E4CB038555AE Authentication-Results: sourceware.org; dmarc=none (p=none dis=none) header.from=gjlay.de Authentication-Results: sourceware.org; spf=none smtp.mailfrom=gjlay.de ARC-Seal: i=1; a=rsa-sha256; t=1690463483; cv=none; d=strato.com; s=strato-dkim-0002; b=U3tHUcxRILOyMuJmB8AMma1+6gntuUH0UPb9HFRd4iYOphoTHu2F/xKn90qe94E5lf VOckad2v6f5N2oC0j/WdzhWyVSUrUmmFk/T23oWjyGTcy0DJbbgOIHYfibRiOk5K36lI qlpipxlJgkwrLwbjY48PsutWfVLLwI3T1yqo6QEZJsAQNFXAC2J+vUcsqbaGq9UVAenE q1h8yfk3hT2ySAoi2atvLFVto5duBLxAclskDw7Q6oLPV2NeZ3hnA/J6daxOW1hx88ZK evBe9mkczmBMQG/FWZ+jNPsJhR/U9ShtgCnwrb3LIccOiI3S9Rb0gIWlMkFyuz8MWZKs 0iJw== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; t=1690463483; s=strato-dkim-0002; d=strato.com; h=In-Reply-To:From:References:Cc:To:Subject:Date:Message-ID:Cc:Date: From:Subject:Sender; bh=VKgWikwYB40cfb3bncTlNHZcqBKPxezBoe/1rPI6zFw=; b=SJIiz3PKmrKHhkq82wSTgHiruOyxC0hmZxOPxTae7p5QKZtwx7n/UWgFpsQ8R9XWPG lPsDXgQgZlLRHiaYIorVo3pZKAG40hqXy4Y1v7Y9t3sDjObcgzn6vytIW8rp8c3SWVEi d4AAPGYUwLjKL5jLagrdCytfkvgwzvpa/8K4pgdZ6fvH3FzxbCMmOg5gSCQyBsSrryin 7TnhhR+JwcEApya7CVsdzI9OMSvQV9h2q10NejgvNvOML9r+Y87S89qz8YZJrEOXwirR ssSIfeohPBI7EvOlWet332AMGvMWEzc/o3M7XxsG3ec+TMF3LKK/7JV5lTY60c9Ov/df a1OA== ARC-Authentication-Results: i=1; strato.com; arc=none; dkim=none X-RZG-CLASS-ID: mo00 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; t=1690463483; s=strato-dkim-0002; d=gjlay.de; h=In-Reply-To:From:References:Cc:To:Subject:Date:Message-ID:Cc:Date: From:Subject:Sender; bh=VKgWikwYB40cfb3bncTlNHZcqBKPxezBoe/1rPI6zFw=; b=ddSmg6ROf54CZxD+Kgc5SVSyRMdFS9gMeqcKfYRJDt7E9X6T1qygLGSxs0c04wikSL E9LphaQK442V38SJvHq4FZEtMUBTqku3EMmhUJV62HhZA9oNkXRfs7O+q9tReGmX0MI9 S8gHudyFfQUHlhUKV8fjWiTTHamyxn3ndXQYiz85MW+u1oFmu6a/MzJprZfxPwCEkNS5 N+GuB5elQOqyQ2dqKZVktrvCuIgHBj8wuQXyIVUUsR+iAq5DZBJlVevUZpaVsv3ldLo1 Un+xygbKd6bZaddpBeF8gxKvwSX7ek2oRoHEe9bAzTt/FFQaI7irtOFMN8Ih9bdHozxG qSHQ== DKIM-Signature: v=1; a=ed25519-sha256; c=relaxed/relaxed; t=1690463483; s=strato-dkim-0003; d=gjlay.de; h=In-Reply-To:From:References:Cc:To:Subject:Date:Message-ID:Cc:Date: From:Subject:Sender; bh=VKgWikwYB40cfb3bncTlNHZcqBKPxezBoe/1rPI6zFw=; b=wIg1EsCEje8FTN/NOraVG6vJt67NET8Ih74R23gkwXBIsBdUuqEeXszOi8Yb2tKLFe 9YgtieDPkDb+BqANq0Bw== X-RZG-AUTH: ":LXoWVUeid/7A29J/hMvvT3koxZnKT7Qq0xotTetVnKkRmM69o2y+LiO3MutATA==" Received: from [192.168.2.102] by smtp.strato.de (RZmta 49.6.6 DYNA|AUTH) with ESMTPSA id Cd29bez6RDBNHnv (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256 bits)) (Client did not present a certificate); Thu, 27 Jul 2023 15:11:23 +0200 (CEST) Message-ID: <9f27de6c-a788-8b1d-59dc-00e8f36c1bd9@gjlay.de> Date: Thu, 27 Jul 2023 15:11:15 +0200 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101 Thunderbird/102.11.0 Subject: Re: LRA for avr: Handling hard regs set directly at expand To: SenthilKumar.Selvaraj@microchip.com, gcc@gcc.gnu.org Cc: vmakarov@redhat.com References: <8e8e6d036d2c00134ac22a577e288c8608a2465c.camel@microchip.com> Content-Language: en-US From: Georg-Johann Lay In-Reply-To: <8e8e6d036d2c00134ac22a577e288c8608a2465c.camel@microchip.com> Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit X-Spam-Status: No, score=-4.4 required=5.0 tests=BAYES_00,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,NICE_REPLY_A,RCVD_IN_DNSWL_LOW,RCVD_IN_MSPIKE_H5,RCVD_IN_MSPIKE_WL,SPF_HELO_PASS,SPF_NONE,TXREP,T_SCC_BODY_TEXT_LINE autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org List-Id: 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). Johann > (define_expand "cpymemhi" > [(parallel [(set (match_operand:BLK 0 "memory_operand" "") > (match_operand:BLK 1 "memory_operand" "")) > (use (match_operand:HI 2 "const_int_operand" "")) > (use (match_operand:HI 3 "const_int_operand" ""))])] > "" > { > if (avr_emit_cpymemhi (operands)) > DONE; > > FAIL; > }) > > where avr_emit_cpymemhi generates > > (insn 14 13 15 4 (set (reg:HI 30 r30) > (reg:HI 48 [ ivtmp.10 ])) "pr53505.c":21:22 -1 > (nil)) > (insn 15 14 16 4 (set (reg:HI 26 r26) > (reg/f:HI 38 virtual-stack-vars)) "pr53505.c":21:22 -1 > (nil)) > (insn 16 15 17 4 (parallel [ > (set (mem:BLK (reg:HI 26 r26) [0 A8]) > (mem:BLK (reg:HI 30 r30) [0 A8])) > (unspec [ > (const_int 0 [0]) > ] UNSPEC_CPYMEM) > (use (reg:QI 52)) > (clobber (reg:HI 26 r26)) > (clobber (reg:HI 30 r30)) > (clobber (reg:QI 0 r0)) > (clobber (reg:QI 52)) > ]) "pr53505.c":21:22 -1 > (nil)) > > Classic reload knows about these - find_reg masks out bad_spill_regs, and bad_spill_regs > when ORed with chain->live_throughout in order_regs_for_reload picks up r30. > > LRA, however, appears to not consider that, and proceeds to use such regs as reload regs. > For the same source, it generates > > Choosing alt 0 in insn 15: (0) =r (1) r {*movhi_split} > Creating newreg=70, assigning class GENERAL_REGS to r70 > 15: r26:HI=r70:HI > REG_EQUAL r28:HI+0x1 > Inserting insn reload before: > 58: r70:HI=r28:HI+0x1 > > Choosing alt 3 in insn 58: (0) d (1) 0 (2) nYnn {*addhi3_split} > Creating newreg=71 from oldreg=70, assigning class LD_REGS to r71 > 58: r71:HI=r71:HI+0x1 > Inserting insn reload before: > 59: r71:HI=r28:HI > Inserting insn reload after: > 60: r70:HI=r71:HI > > ********** Assignment #1: ********** > > Assigning to 71 (cl=LD_REGS, orig=70, freq=3000, tfirst=71, tfreq=3000)... > Assign 30 to reload r71 (freq=3000) > Hard reg 26 is preferable by r70 with profit 1000 > Hard reg 30 is preferable by r70 with profit 1000 > Assigning to 70 (cl=GENERAL_REGS, orig=70, freq=2000, tfirst=70, tfreq=2000)... > Assign 30 to reload r70 (freq=2000) > > > (insn 14 13 59 3 (set (reg:HI 30 r30) > (reg:HI 18 r18 [orig:48 ivtmp.10 ] [48])) "pr53505.c":21:22 101 {*movhi_split} > (nil)) > (insn 59 14 58 3 (set (reg:HI 30 r30 [70]) > (reg/f:HI 28 r28)) "pr53505.c":21:22 101 {*movhi_split} > (nil)) > (insn 58 59 15 3 (set (reg:HI 30 r30 [70]) > (plus:HI (reg:HI 30 r30 [70]) > (const_int 1 [0x1]))) "pr53505.c":21:22 165 {*addhi3_split} > (nil)) > (insn 15 58 16 3 (set (reg:HI 26 r26) > (reg:HI 30 r30 [70])) "pr53505.c":21:22 101 {*movhi_split} > (expr_list:REG_EQUAL (plus:HI (reg/f:HI 28 r28) > (const_int 1 [0x1])) > (nil))) > (insn 16 15 17 3 (parallel [ > (set (mem:BLK (reg:HI 26 r26) [0 A8]) > (mem:BLK (reg:HI 30 r30) [0 A8])) > (unspec [ > (const_int 0 [0]) > ] UNSPEC_CPYMEM) > (use (reg:QI 22 r22 [52])) > (clobber (reg:HI 26 r26)) > (clobber (reg:HI 30 r30)) > (clobber (reg:QI 0 r0)) > (clobber (reg:QI 22 r22 [52])) > ]) "pr53505.c":21:22 132 {cpymem_qi} > (nil)) > > LRA generates insn 59 that clobbers r30 set in insn 14, causing an execution > failure down the line. > > How should the avr backend deal with this? > > Regards > Senthil > >