Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[fork update] 2024-10-24 #3218

Open
wants to merge 5,775 commits into
base: master
Choose a base branch
from

Conversation

CohenArthur
Copy link
Member

@CohenArthur CohenArthur commented Oct 25, 2024

git merge gcc/trunk && git checkout --ours gcc/rust libgrust

  • There are new warnings about narrowing conversions in the borrow checker interface (C++)
  • Some APIs have been changed

@tschwinge
Copy link
Member

git merge gcc/trunk && git checkout --ours gcc/rust libgrust

You can't just drop all upstream changes to gcc/rust/, libgrust/, but have to review them individually.

@tschwinge
Copy link
Member

I suppose it'd help if I wrote down how I'm doing these merges. (..., and sorry for my recent non-activity...)

@CohenArthur
Copy link
Member Author

I suppose it'd help if I wrote down how I'm doing these merges. (..., and sorry for my recent non-activity...)

no worries Thomas! I know you've been busy and gccrs shouldn't be a priority so it's perfectly understandable :) but yes, it'd be extremely helpful if you could write down your process for these.

and btw, I don't think this dropped all changes to gcc/rust - IIUC it only checked out our versions in the case of merge conflicts, which should be okay? this branch has changes that were made upstream but not merged here

@CohenArthur
Copy link
Member Author

so do you take all commits that affect gcc/rust | libgrust | gcc/testsuite/rust from gcc/trunk and merge them individually? how do you apply the rest of the non-Rust commits?

@dkm
Copy link
Member

dkm commented Oct 26, 2024

git merge gcc/trunk && git checkout --ours gcc/rust libgrust

You can't just drop all upstream changes to gcc/rust/, libgrust/, but have to review them individually.

I may have suggested this to @CohenArthur sorry. If we track upstream changes and correctly apply them, that should work (and the traffic from upstream is rather low...).

Do you see other issues with this? We're using something similar (of course with some difference) for the Ada frontend.

@dkm
Copy link
Member

dkm commented Oct 26, 2024

and btw, I don't think this dropped all changes to gcc/rust - IIUC it only checked out our versions in the case of merge conflicts, which should be okay? this branch has changes that were made upstream but not merged here

I think it does drop all changes. It would only drop in case of conflict if you provide a path to a conflicting file. If you give it a path, it will checkout recursively from there.

@CohenArthur
Copy link
Member Author

I'm not sure there's an easy way to find all commits which were applied upstream but not submitted here... we need to do a better job of applying them here as soon as they come in upstream I guess. I'll try and find a list of them

@dkm
Copy link
Member

dkm commented Oct 28, 2024

I'm not sure there's an easy way to find all commits which were applied upstream but not submitted here... we need to do a better job of applying them here as soon as they come in upstream I guess. I'll try and find a list of them

If you have the last commit upstreamed, you should be able to compare the current upstream gcc tree with the old gccrs tree. Or is it too naive?

@CohenArthur
Copy link
Member Author

I think that would work, I was looking at all the differences between the two branches but yeah I should probably look at the differences until the last upstreamed commit

@CohenArthur
Copy link
Member Author

Alright, this is completely different now. I took all of the commits present upstream but not in GitHub, and cherry-picked them separately and fixed all the merge conflicts (by either checkout-ing upstream or our branch depending on the merge conflict's location). Then, I merged gcc/trunk into this branch, and added one commit to fix the very few issues remaining due to API changes upstream and slightly wrong merges. That last commit should probably instead be split up and --fixuped into the original commits, as if they had been committed with a more updated GCC base.

Does that seem alright?

@dkm
Copy link
Member

dkm commented Oct 29, 2024

Alright, this is completely different now. I took all of the commits present upstream but not in GitHub, and cherry-picked them separately and fixed all the merge conflicts (by either checkout-ing upstream or our branch depending on the merge conflict's location). Then, I merged gcc/trunk into this branch, and added one commit to fix the very few issues remaining due to API changes upstream and slightly wrong merges. That last commit should probably instead be split up and --fixuped into the original commits, as if they had been committed with a more updated GCC base.

Does that seem alright?

sounds good, I think. Yes, the last part should be fixedup if you don't want to resolve the conflict again when sending those back in GCC.... But it means rewriting history, which is probably not a good idea. Don't know how @tschwinge handled that ? Or maybe there was no such changes?

@CohenArthur
Copy link
Member Author

Yeah, I was thinking of rewriting our history slightly. It's only one or two commits so I'm thinking it's okay. I don't know if this is what we did as well, but it would avoid build failures upstream for these commits

@CohenArthur
Copy link
Member Author

But in order to autosquash the fixup I'd have to rebase on a veeeeeeeery old commit

@CohenArthur
Copy link
Member Author

However, doing a git rebase -i ... --autosquash produces merge conflicts on the first step... so I'm not sure this is the best way to go about it

@tschwinge
Copy link
Member

tschwinge commented Oct 30, 2024

Yeah, rewriting history for commits that have already git fetched by others is going to cause a lot of pain, please don't do that. (It's fine within the Git branch serving a GitHub pull request, of course.)

@tschwinge
Copy link
Member

Alright, this is completely different now. I took all of the commits present upstream but not in GitHub, and cherry-picked them separately and fixed all the merge conflicts (by either checkout-ing upstream or our branch depending on the merge conflict's location). Then, I merged gcc/trunk into this branch, and added one commit to fix the very few issues remaining due to API changes upstream and slightly wrong merges. That last commit should probably instead be split up and --fixuped into the original commits, as if they had been committed with a more updated GCC base.

Does that seem alright?

No, I suggest to not do it in this way: the cherry-picking will cause every upstream GCC commit to get a new SHA-1 object name (Git commit ID), and therefore be "detached" from its upstream GCC commit, which is likely to cause confusion.

Instead, what I've been doing, is as follows, quickly:

On the GCC/Rust master branch:

$ git log --reverse --stat -p ..upstream/trunk

..., and skip through all commits, until you note something that "requires attention" for GCC/Rust. For example, global changes that require corresponding changes, or upstream commits to gcc/rust/ etc. For such a commit, then merge the commit before (which should merge cleanly, typically...), and then merge the commit itself, and as part of the merge commit, fix up things. Then proceed. Similarly, if running into commits that GCC/Rust has upstreamed: merge the commit before (which should merge cleanly, typically...), and then merge the upstreamed commits with git merge --ours (as discussed elsewhere).

Yes, that's quite a bit of work, and yes, that cannot really be automated very much.

I routinely do the "note" part during my every-day catch-up with GCC upstream for my GCC work branches. (So, I have a text file with all the GCC upstream commits that potentially require attention.) I have failed, in the past several months, to execute this list... :-/

@CohenArthur
Copy link
Member Author

Alright, this is completely different now. I took all of the commits present upstream but not in GitHub, and cherry-picked them separately and fixed all the merge conflicts (by either checkout-ing upstream or our branch depending on the merge conflict's location). Then, I merged gcc/trunk into this branch, and added one commit to fix the very few issues remaining due to API changes upstream and slightly wrong merges. That last commit should probably instead be split up and --fixuped into the original commits, as if they had been committed with a more updated GCC base.
Does that seem alright?

No, I suggest to not do it in this way: the cherry-picking will cause every upstream GCC commit to get a new SHA-1 object name (Git commit ID), and therefore be "detached" from its upstream GCC commit, which is likely to cause confusion.

Instead, what I've been doing, is as follows, quickly:

On the GCC/Rust master branch:

$ git log --reverse --stat -p ..upstream/trunk

..., and skip through all commits, until you note something that "requires attention" for GCC/Rust. For example, global changes that require corresponding changes, or upstream commits to gcc/rust/ etc.

Ah, I guess that part I got right then, by looking at commits which touch gcc/rust | libgrust/ | gcc/testsuite/rust/ upstream that aren't present in our fork.

For such a commit, then merge the commit before (which should merge cleanly, typically...), and then merge the commit itself, and as part of the merge commit, fix up things. Then proceed. Similarly, if running into commits that GCC/Rust has upstreamed: merge the commit before (which should merge cleanly, typically...), and then merge the upstreamed commits with git merge --ours (as discussed elsewhere).

Hmmm... that is a lot of work indeed :< I'm not sure what we could do to make it easier/scriptable and do it more often.

Yes, that's quite a bit of work, and yes, that cannot really be automated very much.

I routinely do the "note" part during my every-day catch-up with GCC upstream for my GCC work branches. (So, I have a text file with all the GCC upstream commits that potentially require attention.) I have failed, in the past several months, to execute this list... :-/

No worries. Honestly, it's a huge burden that shouldn't fall on just one person - hence we should try and improve the process. This isn't something that should rely on you, or occupy a lot of your time. It's quite unfair you've had to deal with this on your own all this time!!!

@thesamesam
Copy link
Contributor

thesamesam commented Oct 30, 2024

Yes, that's quite a bit of work, and yes, that cannot really be automated very much.

Unfortunately, I concur with there not being a better way of doing this, really.

EDIT: Or, well, you could take the Go (or IIRC D) approach, where they have a README that essentially says "don't touch this, send changes to X", and I always interpreted that as a risk of any changes being clobbered on imports.

@dkm
Copy link
Member

dkm commented Oct 31, 2024

Yes, that's quite a bit of work, and yes, that cannot really be automated very much.

Unfortunately, I concur with there not being a better way of doing this, really.

EDIT: Or, well, you could take the Go (or IIRC D) approach, where they have a README that essentially says "don't touch this, send changes to X", and I always interpreted that as a risk of any changes being clobbered on imports.

I must mention that at AdaCore, we are using automated scripts to update several branches from upstream gcc (usually 3), do nightly builds + testsuite runs, with the same goal as Arthur: use a frontend developed on a different gcc base with latest gcc while maintaining both changes from gcc and the main dev (gccrs github). But it's true that changes from upstream to the frontend don't keep their original git rev (but everything is tracked) and there's still some manual tracking to bring changes from gcc to our internal dev branches. And it probably only works smoothly because changes to the frontend coming from upstream gcc are rare (same as gccrs) (wouldn't work for a high traffic FE, or any other central part of the compiler).

cyyself and others added 8 commits October 31, 2024 18:37
When the callee is versioned but the caller is not, we should not inline
the callee into the caller, to prevent the default version of the callee
from being inlined into a not versioned caller.

gcc/ChangeLog:

	* config/riscv/riscv.cc (riscv_can_inline_p): Refuse to inline
	when callee is versioned but caller is not.
…ence-returning functions

As Jakub suggested, use STRIP_REFERENCE_REF instead of doing it manually
as r15-4800-geb828a1e380e7b did.

gcc/cp/ChangeLog:

	* decl.cc (omp_declare_variant_finalize_one): Use STRIP_REFERENCE_REF
	instead of doing it manually.
I have been taking a look at boolean handing once more in the vectorizer.

There are two situation to consider:

  1. when the boolean being created are created from comparing data inputs then
     for the resulting vector boolean we need to know the vector type and the
     precision.  In this case, when we have an operation such as NOT on the data
     element, this has to be lowered to XOR because the truncation to the vector
     precision needs to be explicit.
  2. when the boolean being created comes from another boolean operation, then
     we don't need to lower NOT, as the precision doesn't change.  We don't do
     any lowering for these (as denoted in check_bool_pattern) and instead the
     precision is copied from the element feeding the boolean statement during
     VF analysis.

For early break gcond lowering in order to correctly handle the second scenario
above we punted the lowering of VECT_SCALAR_BOOLEAN_TYPE_P comparisons that were
already in the right shape.  e.g. e != 0 where e is a boolean does not need any
lowering.

The issue however is that the statement feeding e may need to be lowered in the
case where it's a data expression.

This patch changes a bit how we do the lowering.  We now always emit an
additional compare. e.g. if the input is;

  if (e != 0)

where is a boolean we would punt on thi before, but now we generate

  f = e != 0
  if (f != 0)

We then use the same infrastructre as recog_bool to ask it to lower f, and in
doing so handle and boolean conversions that need to be lowered.

Because we now guarantee that f is an internal def we can also simplify the
SLP building code.

When e is a boolean, the precision we build for f needs to reflect the precision
of the operation feeding e.  To get this value we use integer_type_for_mask the
same way recog_bool does, and if it's defined (e.g. we have a data conversions
somewhere) we pass that precision on instead.  This gets us the correct VF
on the newly lowered boolean expressions.

gcc/ChangeLog:

	PR tree-optimization/117176
	* tree-vect-patterns.cc (vect_recog_gcond_pattern): Lower all gconds.
	* tree-vect-slp.cc (vect_analyze_slp): No longer check for in vect def.

gcc/testsuite/ChangeLog:

	PR tree-optimization/117176
	* gcc.dg/vect/vect-early-break_130-pr117176.c: New test.
This was fixed by r12-8835-ge8d5f3a1b5a583 which surely made it latent
but richi points out it was likely an instance of PR90348. -fstack-reuse
continues to be a menace, so let's add the testcase.

gcc/testsuite/ChangeLog:
	PR middle-end/90348
	PR tree-optimization/106073

	* gcc.dg/pr106073.c: New test.
The function body checks for f3 only ran with -mcmodel explicitly set
which meant I missed a regression in my local testing of:

  commit b039d06
  Author: Craig Blackmore <[email protected]>
  Date:   Fri Oct 18 09:17:21 2024 -0600

      [PATCH 3/7] RISC-V: Fix vector memcpy smaller LMUL generation

The failure showed up in the rivos CI and it is due to f3 now using
LMUL m1 instead of m8.

I have reworked the test to make it more robust and maintainable.  This
allowed most of the special casing of command line arguments to be
removed.  It also fixes an issue where some targets would enable
multiple versions of the function body check e.g. `-march=rv32gcv
-mcmodel=medany`.

Changes since v1: Added missing ChangeLog.

gcc/testsuite/ChangeLog:

	* gcc.target/riscv/rvv/base/cpymem-1.c: Fix and rework f3.
Fix some noise seen in "make selftest-valgrind".

gcc/ChangeLog:
	* diagnostic.cc (diagnostic_context::finish): Delete and reset
	m_option_mgr.

Signed-off-by: David Malcolm <[email protected]>
gcc/ChangeLog:
	* opts-diagnostic.cc (output_factory::handler::handler): Use
	std::move on name.

Signed-off-by: David Malcolm <[email protected]>
This patch adds a new class lazy_diagnostic_path for
use when creating rich_location instances, to allow deferring
expensive computations until the path is actually used (when
a diagnostic using the rich_location is emitted).

gcc/ChangeLog:
	* Makefile.in (OBJS): Add lazy-diagnostic-path.o.
	* lazy-diagnostic-path.cc: New file.
	* lazy-diagnostic-path.h: New file.
	* selftest-diagnostic.cc: Include "diagnostic-format.h".
	(test_diagnostic_context::test_diagnostic_context): Turn off
	flushing for the output format's printer.
	* selftest-run-tests.cc (selftest::run_tests): Call
	selftest::lazy_diagnostic_path_cc_tests.
	* selftest.h (selftest::lazy_diagnostic_path_cc_tests): New decl.

Signed-off-by: David Malcolm <[email protected]>
@CohenArthur
Copy link
Member Author

Updated with Thomas' method:

$ git log --format=%h --reverse gcc/trunk ^master -- gcc/rust/ libgrust/ gcc/testsuite/rust/ > log-commits
$ set COMMIT $(cat log-commits | head -$COUNTER | tail -1)
$ set BEFORE $(git log -1 $COMMIT^1 --format=%h)
$ git merge $BEFORE 
$ git merge $COMMIT

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.