This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: PATCH: PR rtl-optimization/54157: [x32] -maddress-mode=long failures


For the record, I can't approve this, but...

"H.J. Lu" <hjl.tools@gmail.com> writes:
> i386,md has
>
> (define_expand "extzv"
>   [(set (match_operand:SI 0 "register_operand")
>         (zero_extract:SI (match_operand 1 "ext_register_operand")
>                          (match_operand:SI 2 "const8_operand")
>                          (match_operand:SI 3 "const8_operand")))]
>   ""
>
> and mode_for_extraction picks word_mode for operand 1 since
> its mode is VOIDmode.  This patch changes mode_for_extraction
> to return the mode of operand 1 if the pattern accepts any mode.
> I added *jcc_btsi_mask_2 since combine now tries a different
> pattern, which leads to test failures on gcc.target/i386/bt-mask-1.c
> and  gcc.target/i386/bt-mask-2.  I didn't update *jcc_btsi_mask_1
> instead since I am not sure if it is used elsewhere.  Tested on
> Linux/x86-64 and Linux/x32.  OK for trunk?

the mode of the extraction operand is defined to be word_mode
for registers (see md.texi), so that at least would need to
be updated.  But I'm not convinced that the wanted_inner_mode here:

   if (! in_dest && unsignedp
-      && mode_for_extraction (EP_extzv, -1) != MAX_MACHINE_MODE)
+      && mode_for_extraction (EP_extzv, -1, VOIDmode) != MAX_MACHINE_MODE)
     {
-      wanted_inner_reg_mode = mode_for_extraction (EP_extzv, 1);
-      pos_mode = mode_for_extraction (EP_extzv, 3);
-      extraction_mode = mode_for_extraction (EP_extzv, 0);
+      wanted_inner_reg_mode = mode_for_extraction (EP_extzv, 1,
+						   inner_mode);
+      pos_mode = mode_for_extraction (EP_extzv, 3, VOIDmode);
+      extraction_mode = mode_for_extraction (EP_extzv, 0, VOIDmode);
     }

is right.  inner_mode is the mode of the thing we're extracting,
which doesn't ncessarily have anything to do with what the ext*
patterns support.

FWIW, in reply to your force_to_mode message, gen_lowpart_for_combine
looks a bit odd:

  if (omode == imode)
    return x;

  /* Return identity if this is a CONST or symbolic reference.  */
  if (omode == Pmode
      && (GET_CODE (x) == CONST
	  || GET_CODE (x) == SYMBOL_REF
	  || GET_CODE (x) == LABEL_REF))
    return x;

So if we know the modes are different, we nevertheless return the
original mode for CONST, SYMBOL_REF or LABEL_REF.  Surely the caller
isn't going to be expecting that?  If we're not prepared to change
the mode to the one that the caller asked for then I think we should
goto fail instead.

I don't know if you're hitting that or not.

Richard


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]