glaubitz added a comment. Hi Rainer!
Thanks for your review. In D90524#2382555 <https://reviews.llvm.org/D90524#2382555>, @ro wrote: > As it happens, I'd arrived at exactly the same patch when I tried a build on > a Debian/sparc64 system in the GCC compile farm (gcc202), submitted as D85582 > <https://reviews.llvm.org/D85582>. > > A couple of questions/caveats: > > - I wouldn't call this patch a workaround, as you do in the summary: if > `clang` cannot find the 32-bit libs, it's broken and needs to be fixed. I'm calling it a workaround because it's already called like that in the existing code comment (well, they call it "hack"): > // 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. > - Please reformat `Linux.cpp` as the pre-merge check requested. OK. > - I'd be willing to accept this patch, but for one I'm not certain about LLVM > policy about who may or may not do so. > - Besides, I believe it's a mistake to split the bug fix and the testsuite > change into to different patches. More about that in D90549 > <https://reviews.llvm.org/D90549>. OK. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D90524/new/ https://reviews.llvm.org/D90524 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
