In D98537#2684910, @myhsu wrote:

In D98537#2682808, @ricky26 wrote:

@myhsu when you get a moment, can you check that this is all OK now? :)

sorry I missed your last update. LGTM now

In D100148#2683710, @hvdijk wrote:

In D100148#2683325, @glaubitz wrote:

Hi! Can we get this patch merged as-is or do we need anything else?

Sorry, miscommunication. I was thinking you could use this to update D99988 to actually build the relevant compiler-rt bits for x32, and then once both that and this are ready, both can be submitted at the same time. The reason for that is that this is not useful by itself, and if it turns out that D99988 actually requires some other clang change as well that we are not seeing yet, it will be easier to update this if it has not been pushed yet. Do you think it would be better to push this first?

Hi! Can we get this patch merged as-is or do we need anything else?

So, we only need D99988 and this one (D100148) now and the LLVM package will finally build on Debian without any additional tweaks.

Thanks, this finally fixes the build for me. I wasn't aware that there was a getArchNameForCompilerRTLib() function in clang.

In D99988#2677123, @hvdijk wrote:

Ah, that looks at first glance like not a compiler-rt problem but a clang problem, it will need to be taught the correct target name for x32 as well. If I'm right, that should be easy to fix, I can take a look later today or tomorrow.

Here is the error:

Ping.

In D99988#2672602, @hvdijk wrote:

I think x32 is not really supported by compiler-rt.

There's definitely support for x32 in libsanitizer, though I do not know how complete it is. libsanitizer is used by GCC as well and I am already using it with GCC on x32 myself.

In D99988#2672582, @hvdijk wrote:

Does this change mean the various *_SUPPORTED_ARCH need updating to list x32 as supported too, or are those handled separately?

In D98884#2671195, @RKSimon wrote:

@glaubitz Now that there's a repro - please could you revert this so @madhur13490 can investigate it offline?

Filed x32-related bug as: https://bugs.llvm.org/show_bug.cgi?id=49861

This change broke the native build on x32 (x86_64 with 32-bit pointers) for me:

In D99869#2668549, @myhsu wrote:

LGTM. Thanks
I don't think we need a second LGTM since this is a minor change

In D52050#2663247, @hvdijk wrote:

In D52050#2663205, @glaubitz wrote:

I think, however, we should bump the rest of the paths to 10.2.0 if possible.

I updated all the Linux trees that were on 4.6.0. The only remaining 4.6.0 trees are for Hurd, which seems to me like it was just a coincidence that it was on the same version. There are other Linux trees used for tests as well, but they were already on different versions of GCC so we do not introduce any new inconsistencies by leaving those as they are.

I noticed that I left out one of the 4.6.0 directories by mistake in what I put up for review, that did not affect test results. I included that as obvious in what I pushed.

In D52050#2662656, @hvdijk wrote:

I have also updated the summary to provide a more complete explanation of the changes, and hope the revised summary will answer @MaskRay's questions.

In D52050#2662440, @hvdijk wrote:

In D52050#2662372, @MaskRay wrote:

Mostly looks good.

clang/test/Driver/Inputs/basic_cross_linux_tree/usr/lib/gcc/x86_64-unknown-linux-gnu/4.6.0/x32/crtbegin.o

Worth bumping the version. 4.6 is quite old and as a host compiler llvm-project has stopped supporting it. The distributions having 4.6 are all end-of-life.
Using a new version (matching your reality) can give impression that the user who contributed the original code may still need it for a while (say 5 years).

Sure, there should be no harm in renaming 4.6.0 to the current version, 10.2.0, will do that and re-test.

In D52050#2660022, @hvdijk wrote:

I think the problem is actually the other thing covered before (don't provide un-normalised triples as cmake arguments), though the wrong detection of LLVM_HOST_TRIPLE will cause other issues too. If we would just update config.guess, the default configuration (not specifying either LLVM_HOST_TRIPLE or LLVM_DEFAULT_TARGET_TRIPLE) should work much better; I have created D99625 for that.

In D52050#2659654, @hvdijk wrote:

I am testing the below, on top of c8e56f394af0b9e32c413d62a0e7aebbba3e6b70, both in a Debian chroot and in my non-Debian system. Initial testing in the Debian chroot suggests that this works for simple cases, clang has no problem finding /usr/lib/gcc/x86_64-linux-gnux32/10, and also picks up the right crt*.o files when using -m32 or -m64. Will do more extensive testing.

In D98574#2654686, @joerg wrote:

The NetBSD part looks ok, but also lacks proper testing. I'm not sure anyone but Linux cares at all otherwise as they lack 32bit SPARC support.

In D52050#2657952, @hvdijk wrote:

I may be missing something, but I do not understand the problem. What systems, other than Debian multi-arch, are you looking to also add support for? My own native x32 system uses (/usr)/libx32 for x32 libraries. Debian uses (/usr)/lib/x86_64-linux-gnux32. I can understand if some people might use (/usr)/lib without any x32 suffix, though I am not aware of anyone doing this. Where does lib32 come from, though? What other systems are you trying to account for?

Updated version which unfortunately still fails.

Gentle ping.

Hmm, there recently were quite some changes in the MultiArch and GCC search path functionality in the Driver and I can unfortunately no longer get it to work on x32.

Updated as I previously forgot to account for FreeBSD as well.

I have updated the patch now with the result that clang should behave as GCC now on Linux, NetBSD and OpenBSD.

For reference, here is what GCC defines on Linux with regards to SPARC:

FWIW, the reason I needed `-mcpu=v9``` was this bug: https://sourceware.org/bugzilla/show_bug.cgi?id=27558

In D98575#2632875, @ro wrote:

Sorry to chime in so late, but I believe this patch has several problems:

• For one, it's at best a partial solution to the problem of atomics on 32-bit SPARC: those are bound to also occur outside of the LLVM tree and this patch does nothing to handle that.

In D98574#2624558, @brad wrote:

I do not immediately see why the other tests are failing, but at a bare minimum the following test from clang/test/Preprocessor/predefined-arch-macros.c will have to be updated..

// RUN: %clang -mcpu=v9 -E -dM %s -o - 2>&1 \
// RUN:     -target sparc-unknown-linux \
// RUN:   | FileCheck -match-full-lines %s -check-prefix=CHECK_SPARC-V9
// CHECK_SPARC-V9-NOT: #define __sparcv8 1
// CHECK_SPARC-V9-NOT: #define __sparcv8__ 1
// CHECK_SPARC-V9: #define __sparc_v9__ 1
// CHECK_SPARC-V9: #define __sparcv9 1
// CHECK_SPARC-V9: #define __sparcv9__ 1

I just realized this change isn't necessary and it also doesn't fix my build problem.

M68k backend has now landed in LLVM. I just rebased this patch and will start the worker in a minute.

Woohoo, finally \o/. Min: Time to push, I guess :D :D :D.

In D88391#2576777, @RKSimon wrote:

@jrtc27 Do you have any more feedback before I accept this and we let the experimental backend be pushed to trunk?

In D52050#2576679, @alsoijw wrote:

In D52050#2369838, @dschuff wrote:

One other question then: do you know if Debian and/or Ubuntu still have the same support for running x32 programs on the regular x86-64 distribution?

I am not sure that I right understand reason of this question, but there is gentoo x32 profile that allow execute all arch x86, x32, x64.

In D88391#2562748, @myhsu wrote:

• Addressed feedbacks

In D88391#2560805, @myhsu wrote:

• [NFC] Addressed most of the feedbacks

In D88389#2547470, @jrtc27 wrote:

Still a number of outstanding style comments from earlier reviews

In D88391#2547311, @RKSimon wrote:

Any more comments? I'm still seeing some style issues but nothing jumps out as critical.

I also want to make sure that everyone is OK as after this is accepted I see no reason why the whole patch series should not get committed.

In D88390#2532131, @rengolin wrote:

In D88390#2531077, @glaubitz wrote:

It's happening what I was hoping and also expecting - external people just coming by and sending such improvements because they love the retro architecture and enjoy working on LLVM.

Ideally, we should get them contributing to the upstream LLVM backend directly, but for that, we need the target in tree.

In D88390#2530878, @myhsu wrote:

In D88390#2530647, @rengolin wrote:

It would be important, however, to know if the m86k community intends to upstream the assembler before moving to production or if the target requires use of third-party closed source tools to work. This will help guide this and other reviews after the first merge, into production.

Speaking of that. Few days ago there is a (draft) PR sent to our GitHub repo: https://github.com/M680x0/M680x0-mono-repo/pull/20
Author of that PR is working on the AsmParser and disassembler (that PR is just a placeholder to inform us in order to prevent overlapping works). I'm pretty excited and looking forward to seeing it appearing upstream before becoming an official backend.

In D88391#2526124, @rengolin wrote:

True, but that was the odd-one-out and we have corrected to conform to the standard for all other back-ends. I explicitly named the m86k upstreaming as the golden rule as to what to do, both from the reviewers and from the authors.

Well, it was quick for C-Sky as well which has several patches already landed: https://github.com/llvm/llvm-project/commits/main/llvm/lib/Target/CSKY

This is the only patch in this series which has not been accepted yet.

In D88385#2498795, @RKSimon wrote:

FWIW I still think we're better off keeping (m68k only) CodeBeads rather than refactoring a lot of code that will affect mainstream targets. After m68k has been pushed, we can reinvestigate whether to keep CodeBeads (and whether other targets would benefit from using it), or moving m68k to a refactored TSFlag mechanism - we can make either a pre-requisite for it losing its experimental status if necessary.

In D88385#2476491, @Paul-C-Anagnostopoulos wrote:

@RKSimon has a good question that I will leave to others to debate.

@craig.topper Actually, a pointer is a bad idea. Instead, if it is necessary to have a separate blob of data, simply use the TSFlags field as an index into a separate table of instances. It's already a uint64_t, which makes a fine index.

I presume the use of this field and the separate table would be triggered by something in a TableGen file, not just hardwired for the M86K target.

In D52050#2468424, @jrtc27 wrote:

However, that's not the same as whether we're on an x86_64 system or on an x32 system determines which GNU triplet to use and which include and library search paths are our primary ones.

But Triple.getEnvironment() will give you llvm::Triple::GNUX32 that you can check?

In D52050#2441164, @glaubitz wrote:

In D52050#2441141, @jrtc27 wrote:

What gets done currently for i386? That suffers potentially the same problem given both /usr/lib/gcc/x86_64-linux-gnu/$V/32 and /usr/lib/gcc/i386-linux-gnu/$V are possible locations for an i386 GCC toolchain.

Very good suggestion. I will look into that tomorrow. Thanks for the pointer!

In D52050#2441141, @jrtc27 wrote:

What gets done currently for i386? That suffers potentially the same problem given both /usr/lib/gcc/x86_64-linux-gnu/$V/32 and /usr/lib/gcc/i386-linux-gnu/$V are possible locations for an i386 GCC toolchain.

In D52050#2441094, @hvdijk wrote:

I've been able to check what Ubuntu 20.10 offers in terms of x32 support. Its kernel supports x32 binaries, it provides x32 versions of core system libraries in separate packages (e.g. libc6-x32, libx32stdc++6, libx32z1), and it provides a compiler that targets x32 by default (gcc-x86-64-linux-gnux32).

In D52050#2428743, @hvdijk wrote:

In D52050#2428735, @glaubitz wrote:

Hmm, I was pretty sure that autoconf can deal with x32 inside an x32 chroot.

Most autoconf-using software won't be dealing with a copy of config.guess that was last updated in 2011. Updating that to a more recent version should also work. :)

In D52050#2428709, @hvdijk wrote:

sys::getDefaultTargetTriple() defaults to LLVM_DEFAULT_TARGET_TRIPLE, which in turn defaults to LLVM_HOST_TRIPLE, which in turn defaults to LLVM_INFERRED_HOST_TRIPLE, which calls config.guess which never automatically detects X32. Sorry, forgot to mention that on my system, I also make sure to explicitly specify LLVM_HOST_TRIPLE to an X32 triple when building LLVM.

The problem seems to be that LLVM_HOST_TRIPLE:STRING is set to x86_64-unknown-linux-gnu instead of `x86_64-unknown-linux-gnux32.

In D52050#2428685, @glaubitz wrote:

I have not found the place yet in clang where to adjust that.

In D52050#2428590, @hvdijk wrote:

That sounds like it's one thing in https://lists.llvm.org/pipermail/llvm-dev/2020-October/146049.html I haven't extracted into a separate patch for submission yet: LLVM does not call Triple::normalize everywhere it should, and misinterprets x86_64-linux-gnux32 as x86_64-somethingelse when it's set as the default target because of it. For testing this change, it may be easiest to just explicitly specify the triple or -mx32.

I stumbled over one problem with my patch which is that when I run an x32 version of clang in a x32 environment, it will still default to "-m64" instead of "-mx32".

Just as a heads-up, I haven't forgotten about this. Freeing up the server just takes a little longer as I need to backup files through a DSL line. Should be ready by tomorrow.

In D52050#2423144, @RKSimon wrote:

Were you able to compare against GCC and also determine debian/ubuntu's x86_64 current support for x32 executables?

Not sure if this is related, but on SPARC, stage2 builds recently started to fail with:

Yeah, @hvdijk has made multiple other improvements which should finally allow the backend to be usable.

Thanks so much! Would you mind pushing that change for me? I don't have commit access at the moment.

I changed it to 4.5 to be consisted with the other Multi-Arch tests for x86, MIPS and PowerPC. That's all.

Thanks a lot Rainer for taking of all of these issues on SPARC!

In D90524#2393746, @ro wrote:

In D90524#2393651, @glaubitz wrote:

Please feel free to reach out to the corresponding debian-$ARCH mailing list for such questions. Debian-specific layouts are not
necessarily obvious at first glance to people not very familiar with the distribution.

I could have. However, my only interest in Linux/SPARC was to have a comparison point for my Solaris/SPARC work to determine which test failures occur on all SPARC targets and which ones are Solaris-specific. I simply don't have to time to dig deeper into Linux issues.

! In D90524#2393635, @ro wrote:

I had an extremely hard time researching the history of directory layouts for my patch D85582. Do as you like, I'm out of this.

In D90524#2393320, @ro wrote:

In D90524#2393312, @glaubitz wrote:

Ping. It would be nice to get this finally merged so that the testsuite noise finally goes down on the sparc64 Linux worker.

Please be a little more patient: one ping a week is considered appropriate, but after only two days is a bit over the top.

In D90524#2393319, @ro wrote:

In D90524#2388214, @glaubitz wrote:

I think it should be good for merging now. I addressed all remarks. I'm still convinced that "workaround" is the proper term though.

Quite the contrary: the comment you cited

// FIXME: This is a bit of a hack. We should really unify this code for
// reasoning about oslibdir spellings with the lib dir spellings in the
// GCCInstallationDetector, but that is a more significant refactoring.

pretty clearly is about how/where support for that layout is implemented in the clang Driver code, not about the layout itself.

Ping. It would be nice to get this finally merged so that the testsuite noise finally goes down on the sparc64 Linux worker.