[Bug target/77333] Incorrect stack adjust in epilogue when targeting i686-w64-mingw32

[jamborm at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=77333

--- Comment #20 from Martin Jambor  ---
Created attachment 40864
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=40864=edit
A small self-contained testcase

I have finally managed to put together a small self-contained
testcase.  Compile with -O2 -fno-ipa-sra to trigger the bug (under
Wine), add -fno-ipa-cp to make it disappear.

[Bug target/77333] Incorrect stack adjust in epilogue when targeting i686-w64-mingw32

[tony at kelman dot net]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=77333

--- Comment #19 from Tony Kelman  ---
The patch also applies and fixes the problem on the gcc-5-branch. I haven't
tried with trunk.

[Bug target/77333] Incorrect stack adjust in epilogue when targeting i686-w64-mingw32

[tony at kelman dot net]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=77333

--- Comment #18 from Tony Kelman  ---
Sorry for the delay in getting to trying out your patch. Yes, it looks like it
does fix the test case here when applied to the gcc 6 branch.

Can running the test suite provide meaningful information from a cross-compile
build?

[Bug target/77333] Incorrect stack adjust in epilogue when targeting i686-w64-mingw32

[jamborm at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=77333

--- Comment #17 from Martin Jambor  ---
OK, so it turns out I botched my testing and the patch from the
previous commit cause PR57330 and we would need something like the
patch below (with a big fat comment why the condition is necessary, if
we go this way at all, that is).  Still, I need someone to test this
really helps to fix the problem and to do a testsuite run.

[PR 77333] Set clone call fntype to callee type

2017-02-03  Martin Jambor  

PR target/77333
* cgraph.c (redirect_call_stmt_to_callee): Set call stmt fntype to
type of the decl if lhs has compatible type.

diff --git a/gcc/cgraph.c b/gcc/cgraph.c
index ef2dc50282c..b91de6788c7 100644
--- a/gcc/cgraph.c
+++ b/gcc/cgraph.c
@@ -1425,7 +1425,15 @@ cgraph_edge::redirect_call_stmt_to_callee (void)
new_stmt = chkp_copy_call_skip_bounds (new_stmt);

   gimple_call_set_fndecl (new_stmt, e->callee->decl);
-  gimple_call_set_fntype (new_stmt, gimple_call_fntype (e->call_stmt));
+
+  tree lhs = gimple_call_lhs (new_stmt);
+  tree decl_restype = TREE_TYPE (TREE_TYPE (e->callee->decl));
+  if (lhs
+ && (VOID_TYPE_P (decl_restype)
+ || !useless_type_conversion_p (TREE_TYPE (lhs), decl_restype)))
+   gimple_call_set_fntype (new_stmt, gimple_call_fntype (e->call_stmt));
+  else
+   gimple_call_set_fntype (new_stmt, TREE_TYPE (e->callee->decl));

   if (gimple_vdef (new_stmt)
  && TREE_CODE (gimple_vdef (new_stmt)) == SSA_NAME)

[Bug target/77333] Incorrect stack adjust in epilogue when targeting i686-w64-mingw32

[jamborm at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=77333

--- Comment #16 from Martin Jambor  ---
Please check whether the following patch (it is against trunk should
apply on top of GCC 6 too) fixes the issue for you.

It can potentially open a whole can of worms so I want to make sure it
helps before taking it further.  Even if it does, I would also like to
ask you to run the whole gcc testsuite (as many languages as you can)
with and without the patch on i686-w64-mingw32 and compare the results
before I propose it on the mailing list.


diff --git a/gcc/cgraph.c b/gcc/cgraph.c
index ef2dc50282c..29ec61b3d0d 100644
--- a/gcc/cgraph.c
+++ b/gcc/cgraph.c
@@ -1425,7 +1425,7 @@ cgraph_edge::redirect_call_stmt_to_callee (void)
new_stmt = chkp_copy_call_skip_bounds (new_stmt);

   gimple_call_set_fndecl (new_stmt, e->callee->decl);
-  gimple_call_set_fntype (new_stmt, gimple_call_fntype (e->call_stmt));
+  gimple_call_set_fntype (new_stmt, TREE_TYPE (e->callee->decl));

   if (gimple_vdef (new_stmt)
  && TREE_CODE (gimple_vdef (new_stmt)) == SSA_NAME)

[Bug target/77333] Incorrect stack adjust in epilogue when targeting i686-w64-mingw32

[tony at kelman dot net]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=77333

--- Comment #15 from Tony Kelman  ---
Created attachment 40608
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=40608=edit
smaller copy of SLPVectorizer.cpp that reproduces issue

[Bug target/77333] Incorrect stack adjust in epilogue when targeting i686-w64-mingw32

[tony at kelman dot net]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=77333

--- Comment #14 from Tony Kelman  ---
I'll look into whether the same flag changes the behavior in the same way on
gcc 5.

Using the opensuse repo's current cross compiler version, I reduced
SLPVectorizer.cpp down to the attached version. It's maybe so small that
there's UB making it useless to look at, but replacing SLPVectorizer.cpp with
this copy reproduces the known-good behavior with -fno-ipa-cp and the segfault
without.

[Bug target/77333] Incorrect stack adjust in epilogue when targeting i686-w64-mingw32

[jamborm at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=77333

Martin Jambor  changed:

   What|Removed |Added

 Target||i686-w64-mingw32
 Status|UNCONFIRMED |NEW
   Last reconfirmed||2017-01-27
   Host||x86_64-linux
 Ever confirmed|0   |1

--- Comment #13 from Martin Jambor  ---
So although I have not gone as far building my own cross-compiler and
just used the gcc 6 based one from OpenSUSE, I managed to reproduce
the issue myself and can confirm that just marking method
SLPVectorizer::tryToVectorizeList with noclone attribute avoids the
issue.

But, from the logs I gather that the only reason why IPA-CP did that
was to remove the unused this pointer.  This makes me wonder whether
this somehow could push its callers and the callee out of sync
regarding what ABI they use for the call?

Apart from that, I have no idea what else could be wrong.  I have
quickly skimmed through the diff of the gimple dumps of the function
before and after cloning and did not see any meaningful difference
(lot's of UIDs were different so something might have escaped me but
since this confirmed my observations from the IPA-CP dump, I doubt
it).

One more thing, IPA-CP does this (clone to remove a parameter when
there are no real constants coming from all callers) only since
r231540 (i.e. gcc 6), so you should not really see the issue with gcc
5, at least not because of this.

[Bug target/77333] Incorrect stack adjust in epilogue when targeting i686-w64-mingw32

[tony at kelman dot net]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=77333

--- Comment #12 from Tony Kelman  ---
Output of dump options uploaded here (8.2 MB file):
https://github.com/tkelman/docker-gcc-bisect/raw/07f6fa56e2f6d60ff90613b9c036f830fb8a422a/LLVMVectorize.tar.gz

[Bug target/77333] Incorrect stack adjust in epilogue when targeting i686-w64-mingw32

[tony at kelman dot net]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=77333

--- Comment #11 from Tony Kelman  ---
Thank you!

-fno-devirtualize on its own did not help.

-fno-ipa-cp on its own did fix the problem.

Adding -fno-ipa-cp only when compiling
lib/Transforms/Vectorize/SLPVectorizer.cpp was enough to fix the problem.
Adding -fsanitize-undefined-trap-on-error as well for that same file didn't
make a difference.

I can post dump output for compiling that same file, if that's what you meant.

In terms of whether or not this is caused by undefined behavior in the source
being compiled, the commit
https://github.com/llvm-mirror/llvm/commit/225dd82d634ca277 made a small
modification to the file in question that prevented the problem from
manifesting. If anything jumps out at you as being definitely undefined
behavior prior to that commit, then we could look into carrying that patch, but
it's very strange that this only happens with this one particular target. That
commit was even intended to not be changing any behavior except when a
newly-added feature is used.

[Bug target/77333] Incorrect stack adjust in epilogue when targeting i686-w64-mingw32

[jamborm at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=77333

--- Comment #10 from Martin Jambor  ---
Interesting to see stack issues after IPA-CP changes.

Please try compiling with -fno-devirtualize

If it does not help, try -fno-ipa-cp (or both).

If any of the above does help, adding it to just the file in which the
miscompiled function is should help, so please verify that.

If devirtualization causes the weird behavior, trying what
-fsanitize-undefined-trap-on-error does might also shed some light on
it.

After we have identified the miscompiled source file and confirmed
that an IPA option causes it, we may try to minimize the testcase or
inspect dumps (-fump-tree-release_ssa -fdump-ipa-all-details
-fdump-tree-fixup_cfg4) for suspect behavior.

Also, we will need to confirm this is indeed a compiler bug and not
source code with undefined behavior.

[Bug target/77333] Incorrect stack adjust in epilogue when targeting i686-w64-mingw32

[tony at kelman dot net]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=77333

--- Comment #9 from Tony Kelman  ---
How can we help get this moving towards resolution? This has kept us stuck on
GCC 4.9, which is getting increasingly problematic. We can attempt to reduce
this to "minimal working piece of opt.exe with gcc 4.9" vs "broken equivalent
piece with gcc 5+," but given the IPA nature of r217587 I'm not sure how that
will go.

[Bug target/77333] Incorrect stack adjust in epilogue when targeting i686-w64-mingw32

[tony at kelman dot net]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=77333

--- Comment #8 from Tony Kelman  ---
`git grep -n deriviz` returns nothing anywhere in the gcc source code
and -fno-derivization gives an error: unrecognized command line option.

[Bug target/77333] Incorrect stack adjust in epilogue when targeting i686-w64-mingw32

[pinskia at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=77333

--- Comment #7 from Andrew Pinski  ---
Try -fno-derivization (I think that is the spelling).

[Bug target/77333] Incorrect stack adjust in epilogue when targeting i686-w64-mingw32

[tony at kelman dot net]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=77333

--- Comment #6 from Tony Kelman  ---
Created attachment 39590
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=39590=edit
bisect-log.txt

[Bug target/77333] Incorrect stack adjust in epilogue when targeting i686-w64-mingw32

[tony at kelman dot net]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=77333

--- Comment #5 from Tony Kelman  ---
Created attachment 39589
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=39589=edit
bisect-run.sh

[Bug target/77333] Incorrect stack adjust in epilogue when targeting i686-w64-mingw32

[tony at kelman dot net]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=77333

--- Comment #4 from Tony Kelman  ---
I managed to bisect this to SVN revision 217587 by
Martin Jambor  using the attached
Dockerfile and bisect-run.sh. This was cross-compiling
from openSUSE 42.1 which happened to have most of the
build-deps easily available but should be reproducible
with a cross-compiler most places you can get wine.

[Bug target/77333] Incorrect stack adjust in epilogue when targeting i686-w64-mingw32

[tony at kelman dot net]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=77333

--- Comment #3 from Tony Kelman  ---
Created attachment 39588
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=39588=edit
Dockerfile

[Bug target/77333] Incorrect stack adjust in epilogue when targeting i686-w64-mingw32

[kfischer at college dot harvard.edu]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=77333

--- Comment #2 from Keno Fischer  ---
Still broken adding both options to the compile.

[Bug target/77333] Incorrect stack adjust in epilogue when targeting i686-w64-mingw32

[rguenth at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=77333

--- Comment #1 from Richard Biener  ---
Try -fno-lifetime-dse and/or -fno-delete-null-pointer-checks.