Thanks Jeff, I will resolve the conflict and send v3 after test. Pan
-----Original Message----- From: Jeff Law <jeffreya...@gmail.com> Sent: Monday, January 27, 2025 12:38 AM To: Li, Pan2 <pan2...@intel.com>; gcc-patches@gcc.gnu.org Cc: juzhe.zh...@rivai.ai; kito.ch...@gmail.com; rdapp....@gmail.com Subject: Re: [PATCH v2 1/4] RISC-V: Refactor SAT_* operand rtx extend to reg help func [NFC] On 1/23/25 12:01 AM, pan2...@intel.com wrote: > From: Pan Li <pan2...@intel.com> > > This patch would like to refactor the helper function of the SAT_* > scalar. The helper function will convert the define_pattern ops > to the xmode reg for the underlying code-gen. This patch add > new parameter for ZERO_EXTEND or SIGN_EXTEND if the input is const_int > or the mode is non-Xmode. > > The below test suites are passed for this patch. > * The rv64gcv fully regression test. > > gcc/ChangeLog: > > * config/riscv/riscv.cc (riscv_gen_zero_extend_rtx): Rename from ... > (riscv_extend_to_xmode_reg): Rename to and add rtx_code for > zero/sign extend if non-Xmode. > (riscv_expand_usadd): Leverage the renamed function with ZERO_EXTEND. > (riscv_expand_ussub): Ditto. Note that I recently made a small change to riscv_gen_zero_extend_rtx that I think you need to incorporate into your patch. Otherwise I think you'll get a code quality regression on some of the saturation tests. Combine purposefully doesn't try to simplify expressions like (zero_extend (const_int ...)) or (sign_extend (const_int ...)). It's a historical wart, probably related to the lack of a mode on const_int objects IIRC. As a result if you ask the old code for an SImode 0x80000000 you get a load of 0xffffffff80000000 (lui) followed by a zero extend (two shifts). What you really want is a li to load 0x1, then a left shift. to produce 0x80000000. This problem had been previously masked by the mvconst_internal pattern. The way to get this behavior is to take the incoming constant and mask off all the bits outside the desired mode. Then use gen_int_mode to actually generate a canonical const_int. Then force that into a register with force_reg. ie: > /* Combine deliberately does not simplify extensions of constants > (long story). So try to generate the zero extended constant > efficiently. > > First extract the constant and mask off all the bits not in MODE. */ > HOST_WIDE_INT val = INTVAL (x); > val &= GET_MODE_MASK (mode); > > /* X may need synthesis, so do not blindly copy it. */ > xmode_reg = force_reg (Xmode, gen_int_mode (val, Xmode)); I think the upstream ci system hasn't moved the baseline forward in about a week. As a result it's not reporting the failure to apply due to the conflict nor is it reporting the code quality regression. Jeff
