Skip to content

ci: Compile with checking enabled #4002

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

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

powerboat9
Copy link
Collaborator

No description provided.

It looks like the sourceware runners are compiling with checking, and
that may be why they error out where we don't. Regardless, this should
make our CI more sensitive to bugs.

ChangeLog:

	* .github/workflows/ccpp.yml: Configure with
	--enable-checking=yes.
	* .github/workflows/ccpp32alpine.yml: Likewise.

Signed-off-by: Owen Avery <[email protected]>
@powerboat9
Copy link
Collaborator Author

This doesn't help resolve https://builder.sourceware.org/buildbot/#/builders/27 like I was hoping, but it should still be nice to have.

Copy link
Member

@tschwinge tschwinge left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Build of multilibs is enabled by default, and also checking (yes,extra) is enabled by default, gcc/configure.ac:

# Determine the default checks.                                                                                                                                                                                                               
if test x$is_release = x ; then                                                                                                                                                                                                               
  ac_checking_flags=yes,extra                                                                                                                                                                                                                 
else                                                                                                                                                                                                                                          
  ac_checking_flags=release                                                                                                                                                                                                                   
fi])                                                                                                                                                                                                                                          

@tschwinge
Copy link
Member

The fails are for a 32-bit x86 host system. We have .github/workflows/ccpp32alpine.yml, so what is different here, hmm...

@tschwinge
Copy link
Member

Haha! "GCC Rust build and test (Alpine 32-bit) / build-alpine-32bit-and-check-alpine-32bit" produced:

UNSUPPORTED: rust/execute/torture/sip-hasher.rs   -O0 
UNSUPPORTED: rust/execute/torture/sip-hasher.rs   -O1 
UNSUPPORTED: rust/execute/torture/sip-hasher.rs   -O2 
UNSUPPORTED: rust/execute/torture/sip-hasher.rs   -O3 -g 
UNSUPPORTED: rust/execute/torture/sip-hasher.rs   -Os 
UNSUPPORTED: rust/execute/torture/sip-hasher.rs   -O2 -flto -fno-use-linker-plugin -flto-partition=none 
UNSUPPORTED: rust/execute/torture/sip-hasher.rs   -O2 -flto -fuse-linker-plugin -fno-fat-lto-objects 

..., because of // { dg-skip-if "" { *-*-* } { "-m32" } { "" } }, and this testing here is running with explicit -m32 (nothing wrong with that). That's why we don't see the internal compiler error: in expand_shift_1, at expmed.cc:2713 here.

That test case was added in #3915, #1247 commit 9526906 "gccrs: fix bad monomophization of generic paths". @philberty: What's the rationale behind the -m32 skip? If anything, this should skip for effective-target ilp32 (?) or only allow for lp64 (?), but that all sounds dubious, papering over an actual problem -- like this ICE (I suppose?), so yet better, instead of dg-skip-if, this should (conditionally) mark up the ICE with dg-ice.

@philberty
Copy link
Member

@tschwinge this is my excitement where this goal test case now works but its uncovered a bug with how we do the intrinsic u32 rotate which is causing this ICE on m32

I think the issue as far as i can tell is we need to change the shift count to be masked by the size of the type but it was an related change to that patch. I'll open an issue

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.

3 participants