diff mbox series

gcc: m68k: fix PR target/107645

Message ID 20221111194356.3922768-1-jcmvbkbc@gmail.com
State New
Headers show
Series gcc: m68k: fix PR target/107645 | expand

Commit Message

Max Filippov Nov. 11, 2022, 7:43 p.m. UTC 
gcc/
	PR target/107645
	* config/m68k/predicates.md (symbolic_operand): Return false
	when UNSPEC is under the CONST node.
---
Regtested with --enable-checking=all for target=m68k-linux-uclibc, no
new regressions compared to the compiler built without checking.
Ok for master?

 gcc/config/m68k/predicates.md | 2 ++
 1 file changed, 2 insertions(+)

Comments

Jeff Law Nov. 12, 2022, 7:42 p.m. UTC | #1 
On 11/11/22 12:43, Max Filippov wrote:
> gcc/
> 	PR target/107645
> 	* config/m68k/predicates.md (symbolic_operand): Return false
> 	when UNSPEC is under the CONST node.

Isn't the underlying object still symbolic though, thus returning false 
seems wrong.

insn 342 341 343 35 (set (reg:SI 97)
         (mem/u:SI (plus:SI (reg:SI 13 %a5)
                 (const:SI (unspec:SI [
                             (symbol_ref:SI 
("__gcov_kvp_dynamic_pool_index") [flags 0x40]  <var_decl 0x7f9e83fe7cf0 
__gcov_kvp_dynamic_pool_index>)
                             (const_int 0 [0])
                         ] 6))) [0  S4 A8])) 
"gcc/libgcc/libgcov.h":472:44 55 {*movsi_m68k2}
      (expr_list:REG_EQUAL (symbol_ref:SI 
("__gcov_kvp_dynamic_pool_index") [flags 0x40]  <var_decl 0x7f9e83fe7cf0 
__gcov_kvp_dynamic_pool_index>)
         (nil)))


ISTM that we'd need to strip the unspec and process its argument 
instead.  But maybe I'm missing something.


jeff
Max Filippov Nov. 13, 2022, 3:47 a.m. UTC | #2 
On Sat, Nov 12, 2022 at 11:42 AM Jeff Law <jeffreyalaw@gmail.com> wrote:
> ISTM that we'd need to strip the unspec and process its argument
> instead.

I tried that first, the result was more ICEs because that pattern
wasn't recognized at later stages. Then I read the change to the
comment over the symbolic_operand predicate:
https://gcc.gnu.org/git/?p=gcc.git;a=blobdiff;f=gcc/config/m68k/predicates.md;h=6ca261fb92a2b7ecd53a0356d06410e2c0d70965;hp=417989f6d6c408fa82af9f9649a204b9a754d1dc;hb=75df395f15f2;hpb=676fd528c9990a4f1046b51d40059893c3a71490
and that made me think that the intention was to not recognize
the unspecs in that predicate.
Jeff Law Nov. 16, 2022, 3:33 a.m. UTC | #3 
On 11/12/22 20:47, Max Filippov wrote:
> On Sat, Nov 12, 2022 at 11:42 AM Jeff Law <jeffreyalaw@gmail.com> wrote:
>> ISTM that we'd need to strip the unspec and process its argument
>> instead.
> I tried that first, the result was more ICEs because that pattern
> wasn't recognized at later stages. Then I read the change to the
> comment over the symbolic_operand predicate:
> https://gcc.gnu.org/git/?p=gcc.git;a=blobdiff;f=gcc/config/m68k/predicates.md;h=6ca261fb92a2b7ecd53a0356d06410e2c0d70965;hp=417989f6d6c408fa82af9f9649a204b9a754d1dc;hb=75df395f15f2;hpb=676fd528c9990a4f1046b51d40059893c3a71490
> and that made me think that the intention was to not recognize
> the unspecs in that predicate.

Thanks for clarifying.  ISTM that operand predicate is quite poorly named.


OK for the trunk.


jeff
Max Filippov Nov. 16, 2022, 12:37 p.m. UTC | #4 
On Tue, Nov 15, 2022 at 7:33 PM Jeff Law <jeffreyalaw@gmail.com> wrote:
> Thanks for clarifying.  ISTM that operand predicate is quite poorly named.
>
> OK for the trunk.

Thanks. Applied to master.

-- Max
diff mbox series

Patch

diff --git a/gcc/config/m68k/predicates.md b/gcc/config/m68k/predicates.md
index f8dedd9f8375..094a18955534 100644
--- a/gcc/config/m68k/predicates.md
+++ b/gcc/config/m68k/predicates.md
@@ -141,6 +141,8 @@ 
 
     case CONST:
       op = XEXP (op, 0);
+      if (GET_CODE (op) == UNSPEC)
+        return false;
       return ((GET_CODE (XEXP (op, 0)) == SYMBOL_REF
 	       || GET_CODE (XEXP (op, 0)) == LABEL_REF)
 	      && GET_CODE (XEXP (op, 1)) == CONST_INT);