LGTM

LGTM

LGTM

In D69814#2397954, @jroelofs wrote:

Since this adds a dependency on the AAResultsWrapperPass, shouldn't we be informing the pass manager about it?

diff --git a/llvm/lib/CodeGen/MachineScheduler.cpp b/llvm/lib/CodeGen/MachineScheduler.cpp
index 256628a179ae..950c87ea26f9 100644
--- a/llvm/lib/CodeGen/MachineScheduler.cpp
+++ b/llvm/lib/CodeGen/MachineScheduler.cpp
@@ -240,8 +240,11 @@ char PostMachineScheduler::ID = 0;
 
 char &llvm::PostMachineSchedulerID = PostMachineScheduler::ID;
 
-INITIALIZE_PASS(PostMachineScheduler, "postmisched",
-                "PostRA Machine Instruction Scheduler", false, false)
+INITIALIZE_PASS_BEGIN(PostMachineScheduler, "postmisched",
+                      "PostRA Machine Instruction Scheduler", false, false)
+INITIALIZE_PASS_DEPENDENCY(AAResultsWrapperPass)
+INITIALIZE_PASS_END(PostMachineScheduler, "postmisched",
+                    "PostRA Machine Instruction Scheduler", false, false)
 
 PostMachineScheduler::PostMachineScheduler() : MachineSchedulerBase(ID) {
   initializePostMachineSchedulerPass(*PassRegistry::getPassRegistry());

Since this adds a dependency on the AAResultsWrapperPass, shouldn't we be informing the pass manager about it?

Seems reasonable.

More context: https://www.openwall.com/lists/musl/2013/03/29/13

In D13673#2394064, @ldionne wrote:

@vkalintiris @jroelofs Is it possible to detect Musl through some macro? I'd like to get rid of the CMake option -- this isn't the sort of property that we want to set explicitly at configure time, it's the sort of property we want to sniff out in the library instead.

👍

There's quite a few dead variables added in this patch. Do they belong in later patches in the series?

LGTM

LGTM

LGTM

small nits, otherwise LGTM

update LGTM too

btw, do you need me to commit it for you?

Yeah, this should do what you're after. LGTM

LGTM

LGTM, with some optional suggestions.

LGTM

LGTM

I know almost nothing about how DebugInfo is represented in llvm/dwarf, but is there not a way to represent what this transformation is doing in DI? It's always a little sad when you have to drop it, since doing so makes the debug experience worse.

LGTM

LGTM

LGTM

LGTM

LGTM

LGTM

LGTM

In D87164#2268366, @mcarrasco wrote:

@jroelofs no worries, thanks a lot for your time.

Please tell me if I understood you correctly.

It is acceptable just to append the -lgcc if -rtlib=libgcc is used, although the user is responsible to ensure that correct paths are provided so libgcc is found.

LGTM

LGTM

LGTM

LGTM

The Baremetal toolchain currently has a fixed behavior regardless of the -rtlib option's value. It is always linking against compiler-rt unless -nodefaultlibs is enabled.

This proposal slightly changes the code in such a way that enabling -rtlib=libgcc implies linking against -libgcc.

Something that I'm unsure about this patch is that we are not enforcing the existence of such libgcc. By reading other toolchains, I understood that is not always enforced. Do you think this policy is acceptable? If it is not, how would you think it should be addressed?

b15f2bd3ad4ab614e75ff859b03c3c1eb9f1a9d3

I was *just* thinking about the inverse of this transformation as a way to increase available ILP on CPUs where there are more pipes that can do arithmetic than compares.... cool!

I'm having trouble getting -stop-before MIR output to trigger these remarks when fed back into llc, so instead I've just relaxed the checks against specific cycle counts to be more forgiving.

In D86797#2244962, @thegameg wrote:

This looks great, thanks! Since it's a CodeGen pass, I would suggest using a MIR test instead of an IR one, but I think it's probably easier to read it as it is right now.

In D85004#2188062, @danielkiss wrote:

Added here as I see:
https://github.com/llvm/llvm-project/blob/19bc9ea480b60b607a3e303f20c7a3a2ea553369/libcxxabi/test/CMakeLists.txt#L57

Isn't it missing DEPENDS cxxabi the same way this was missing DEPENDS unwind?

Mind doing the same thing for libcxxabi? I think it has the same problem.

LGTM, thank you!

@njames93 I believe this change broke one of the buildbots (verified by reverting it locally). Mind taking a look? From a quick glance, I can't tell where the functional change is here.