Message ID | 20240927075111.208305-1-pan2.li@intel.com |
---|---|
State | New |
Headers | show |
Series | [v1] Widening-Mul: Fix one ICE when iterate on phi node | expand |
On Fri, Sep 27, 2024 at 9:52 AM <pan2.li@intel.com> wrote: > > From: Pan Li <pan2.li@intel.com> > > We iterate all phi node of bb to try to match the SAT_* pattern > for scalar integer. We also remove the phi mode when the relevant > pattern matched. > > Unfortunately the iterator may have no idea the phi node is removed > and continue leverage the free data and then ICE similar as below. > > [0] psi ptr 0x75216340c000 > [0] psi ptr 0x75216340c400 > [1] psi ptr 0xa5a5a5a5a5a5a5a5 <=== GC freed pointer. > > during GIMPLE pass: widening_mul > tmp.c: In function ‘f’: > tmp.c:45:6: internal compiler error: Segmentation fault > 45 | void f(int rows, int cols) { > | ^ > 0x36e2788 internal_error(char const*, ...) > ../../gcc/diagnostic-global-context.cc:517 > 0x18005f0 crash_signal > ../../gcc/toplev.cc:321 > 0x752163c4531f ??? > ./signal/../sysdeps/unix/sysv/linux/x86_64/libc_sigaction.c:0 > 0x103ae0e bool is_a_helper<gphi*>::test<gimple>(gimple*) > ../../gcc/gimple.h:1256 > 0x103f9a5 bool is_a<gphi*, gimple>(gimple*) > ../../gcc/is-a.h:232 > 0x103dc78 gphi* as_a<gphi*, gimple>(gimple*) > ../../gcc/is-a.h:255 > 0x104f12e gphi_iterator::phi() const > ../../gcc/gimple-iterator.h:47 > 0x1a57bef after_dom_children > ../../gcc/tree-ssa-math-opts.cc:6140 > 0x3344482 dom_walker::walk(basic_block_def*) > ../../gcc/domwalk.cc:354 > 0x1a58601 execute > ../../gcc/tree-ssa-math-opts.cc:6312 > > This patch would like to fix the iterate on modified collection problem > by backup the next phi in advance. > > The below test suites are passed for this patch. > * The rv64gcv fully regression test. > * The x86 bootstrap test. > * The x86 fully regression test. OK. Thanks, Richard. > PR middle-end/116861 > > gcc/ChangeLog: > > * tree-ssa-math-opts.cc (math_opts_dom_walker::after_dom_children): Backup > the next psi iterator before remove the phi node. > > gcc/testsuite/ChangeLog: > > * gcc.dg/torture/pr116861-1.c: New test. > > Signed-off-by: Pan Li <pan2.li@intel.com> > --- > gcc/testsuite/gcc.dg/torture/pr116861-1.c | 76 +++++++++++++++++++++++ > gcc/tree-ssa-math-opts.cc | 9 ++- > 2 files changed, 83 insertions(+), 2 deletions(-) > create mode 100644 gcc/testsuite/gcc.dg/torture/pr116861-1.c > > diff --git a/gcc/testsuite/gcc.dg/torture/pr116861-1.c b/gcc/testsuite/gcc.dg/torture/pr116861-1.c > new file mode 100644 > index 00000000000..7dcfe664d89 > --- /dev/null > +++ b/gcc/testsuite/gcc.dg/torture/pr116861-1.c > @@ -0,0 +1,76 @@ > +/* { dg-do compile } */ > +/* { dg-options "-O2" } */ > + > +void pm_message(void); > +struct CmdlineInfo { > + _Bool wantCrop[4]; > + unsigned int margin; > +}; > +typedef struct { > + unsigned int removeSize; > +} CropOp; > +typedef struct { > + CropOp op[4]; > +} CropSet; > +static void divideAllBackgroundIntoBorders(unsigned int const totalSz, > + _Bool const wantCropSideA, > + _Bool const wantCropSideB, > + unsigned int const wantMargin, > + unsigned int *const sideASzP, > + unsigned int *const sideBSzP) { > + unsigned int sideASz, sideBSz; > + if (wantCropSideA && wantCropSideB) > + { > + sideASz = totalSz / 2; > + if (wantMargin) > + sideBSz = totalSz - sideASz; > + } > + else if (wantCropSideB) > + { > + sideBSz = 0; > + } > + *sideASzP = sideASz; > + *sideBSzP = sideBSz; > +} > +static CropOp oneSideCrop(_Bool const wantCrop, unsigned int const borderSz, > + unsigned int const margin) { > + CropOp retval; > + if (wantCrop) > + { > + if (borderSz >= margin) > + retval.removeSize = borderSz - margin; > + else > + retval.removeSize = 0; > + } > + return retval; > +} > +struct CmdlineInfo cmdline1; > +void f(int rows, int cols) { > + struct CmdlineInfo cmdline0 = cmdline1; > + CropSet crop; > + struct CmdlineInfo cmdline = cmdline0; > + CropSet retval; > + unsigned int leftBorderSz, rghtBorderSz; > + unsigned int topBorderSz, botBorderSz; > + divideAllBackgroundIntoBorders(cols, cmdline.wantCrop[0], > + cmdline.wantCrop[1], cmdline.margin > 0, > + &leftBorderSz, &rghtBorderSz); > + divideAllBackgroundIntoBorders(rows, cmdline.wantCrop[2], > + cmdline.wantCrop[3], cmdline.margin > 0, > + &topBorderSz, &botBorderSz); > + retval.op[0] = > + oneSideCrop(cmdline.wantCrop[0], leftBorderSz, cmdline.margin); > + retval.op[1] = > + oneSideCrop(cmdline.wantCrop[1], rghtBorderSz, cmdline.margin); > + retval.op[2] = > + oneSideCrop(cmdline.wantCrop[2], topBorderSz, cmdline.margin); > + retval.op[3] = > + oneSideCrop(cmdline.wantCrop[3], botBorderSz, cmdline.margin); > + crop = retval; > + unsigned int i = 0; > + for (i = 0; i < 4; ++i) > + { > + if (crop.op[i].removeSize == 0) > + pm_message(); > + } > +} > diff --git a/gcc/tree-ssa-math-opts.cc b/gcc/tree-ssa-math-opts.cc > index 8c622514dbd..f1cfe7dfab0 100644 > --- a/gcc/tree-ssa-math-opts.cc > +++ b/gcc/tree-ssa-math-opts.cc > @@ -6129,10 +6129,15 @@ math_opts_dom_walker::after_dom_children (basic_block bb) > > fma_deferring_state fma_state (param_avoid_fma_max_bits > 0); > > - for (gphi_iterator psi = gsi_start_phis (bb); !gsi_end_p (psi); > - gsi_next (&psi)) > + for (gphi_iterator psi_next, psi = gsi_start_phis (bb); !gsi_end_p (psi); > + psi = psi_next) > { > + psi_next = psi; > + gsi_next (&psi_next); > + > gimple_stmt_iterator gsi = gsi_after_labels (bb); > + > + /* The match_* may remove phi node. */ > match_saturation_add (&gsi, psi.phi ()); > match_unsigned_saturation_sub (&gsi, psi.phi ()); > } > -- > 2.43.0 >
diff --git a/gcc/testsuite/gcc.dg/torture/pr116861-1.c b/gcc/testsuite/gcc.dg/torture/pr116861-1.c new file mode 100644 index 00000000000..7dcfe664d89 --- /dev/null +++ b/gcc/testsuite/gcc.dg/torture/pr116861-1.c @@ -0,0 +1,76 @@ +/* { dg-do compile } */ +/* { dg-options "-O2" } */ + +void pm_message(void); +struct CmdlineInfo { + _Bool wantCrop[4]; + unsigned int margin; +}; +typedef struct { + unsigned int removeSize; +} CropOp; +typedef struct { + CropOp op[4]; +} CropSet; +static void divideAllBackgroundIntoBorders(unsigned int const totalSz, + _Bool const wantCropSideA, + _Bool const wantCropSideB, + unsigned int const wantMargin, + unsigned int *const sideASzP, + unsigned int *const sideBSzP) { + unsigned int sideASz, sideBSz; + if (wantCropSideA && wantCropSideB) + { + sideASz = totalSz / 2; + if (wantMargin) + sideBSz = totalSz - sideASz; + } + else if (wantCropSideB) + { + sideBSz = 0; + } + *sideASzP = sideASz; + *sideBSzP = sideBSz; +} +static CropOp oneSideCrop(_Bool const wantCrop, unsigned int const borderSz, + unsigned int const margin) { + CropOp retval; + if (wantCrop) + { + if (borderSz >= margin) + retval.removeSize = borderSz - margin; + else + retval.removeSize = 0; + } + return retval; +} +struct CmdlineInfo cmdline1; +void f(int rows, int cols) { + struct CmdlineInfo cmdline0 = cmdline1; + CropSet crop; + struct CmdlineInfo cmdline = cmdline0; + CropSet retval; + unsigned int leftBorderSz, rghtBorderSz; + unsigned int topBorderSz, botBorderSz; + divideAllBackgroundIntoBorders(cols, cmdline.wantCrop[0], + cmdline.wantCrop[1], cmdline.margin > 0, + &leftBorderSz, &rghtBorderSz); + divideAllBackgroundIntoBorders(rows, cmdline.wantCrop[2], + cmdline.wantCrop[3], cmdline.margin > 0, + &topBorderSz, &botBorderSz); + retval.op[0] = + oneSideCrop(cmdline.wantCrop[0], leftBorderSz, cmdline.margin); + retval.op[1] = + oneSideCrop(cmdline.wantCrop[1], rghtBorderSz, cmdline.margin); + retval.op[2] = + oneSideCrop(cmdline.wantCrop[2], topBorderSz, cmdline.margin); + retval.op[3] = + oneSideCrop(cmdline.wantCrop[3], botBorderSz, cmdline.margin); + crop = retval; + unsigned int i = 0; + for (i = 0; i < 4; ++i) + { + if (crop.op[i].removeSize == 0) + pm_message(); + } +} diff --git a/gcc/tree-ssa-math-opts.cc b/gcc/tree-ssa-math-opts.cc index 8c622514dbd..f1cfe7dfab0 100644 --- a/gcc/tree-ssa-math-opts.cc +++ b/gcc/tree-ssa-math-opts.cc @@ -6129,10 +6129,15 @@ math_opts_dom_walker::after_dom_children (basic_block bb) fma_deferring_state fma_state (param_avoid_fma_max_bits > 0); - for (gphi_iterator psi = gsi_start_phis (bb); !gsi_end_p (psi); - gsi_next (&psi)) + for (gphi_iterator psi_next, psi = gsi_start_phis (bb); !gsi_end_p (psi); + psi = psi_next) { + psi_next = psi; + gsi_next (&psi_next); + gimple_stmt_iterator gsi = gsi_after_labels (bb); + + /* The match_* may remove phi node. */ match_saturation_add (&gsi, psi.phi ()); match_unsigned_saturation_sub (&gsi, psi.phi ()); }