LGTM. I agree this is a good change, delta passes shouldn't try to be too clever.

(I actually wanted the "Don't require any other arguments for --print-delta-passes" just the other day)

llvm-reduce change is LGTM

ok the llvm-reduce change looks clearly good. I'm not the right person to look at the other part

omg thank you, the cloning bug(s) have been bothering me for a long time

In D138953#3965051, @arsenm wrote:

In D138953#3958442, @regehr wrote:

just fyi, the experience with this from C-Reduce is that first, killed compilers will tend to leave crap in /tmp or similar, and this builds up over time, but usually not that quickly.

opt and llc don't do this, so I'm not too worried about this. Not sure what I would try to do about this anyway since tools tend to make up their own random unique filenames.

LGTM!

just fyi, the experience with this from C-Reduce is that first, killed compilers will tend to leave crap in /tmp or similar, and this builds up over time, but usually not that quickly.

In D136476#3875502, @arsenm wrote:

My idea of simpler to understand is codegens to fewer instructions. Things that introduce undefined behavior are more likely to enable optimizations and different lowerings that reduce instructions emitted

there's a simple test that we can apply to determine whether we add or remove any given flag: we should do whatever reduces the amount of undefined behavior. so, for example, nsw is one that we definitely to eliminate because this makes code more defined and hence (in general) easier to understand.

LGTM

so at some level here we're scoring a test case based on how simple it is for a human being to understand. I mean, that's what we should be doing. from that point of view, I dislike assigning a small cost to poison and undef -- they're probably the most semantically challenging concepts in this entire IR, in terms of the number of compiler bugs they have caused.

In D131920#3724694, @aeubanks wrote:

looks like the landed commit 2f1fa6242a1fff0743f74b67f9ea6c08259acfa1 didn't contain phab's summary in its commit message

I like this -- especially the part that gets rid of the interestingness test's output

I like the debug output and I also like making more test cases abort on invalid IR, we definitely have some bugs to flush out here.

LGTM

LGTM but happy to wait for Matt's view.

here's a proposal: if we decide to go forward with my patch instead of Arthur's, I'll do a bit of work to remove the passes one by one in order to help us make a more informed decision about which passes we should be running here.

In D128470#3701107, @arsenm wrote:

Overlap with D123749?

this patch is currently failing some tests but I can fix that up quickly. I just benchmarked it and it reduces final file size by 7% on my test cases. not earthshattering, but the reduced files end up being somewhat more pleasant to look at, since the passes leave things looking nice and canonical. anyone have time to look this over?

ok hopefully final version, sorry, getting started slowly this morning

better now

updated patch with code suggested by matt, thanks, this is much better

this is the first version of llvm-reduce I've seen that makes it all the way through my benchmark suite in --abort-on-invalid-reduction mode!

fixed via https://github.com/llvm/llvm-project/commit/5b4f6d8b4b861cad94bf85e9492ac9b1fb4e15e2

address comment by Matt

In D131026#3695477, @arsenm wrote:

LGTM. I do think something is off about how the pass currently tries to auto-fold unconditional branches to successors. I was thinking there should be a separate control flow simplification to clean those up

thanks Arthur-- I've added this test and also refined the safety check in the pass to be less conservative

closed via https://github.com/llvm/llvm-project/commit/71d1bd145724ea81c35c8811e74fff8402ea8e3a

I definitely support renaming any inaccurately named tests to be more accurate

Also, notice that this patch takes a bunch of program points that introduce undef and makes them all point to a getDefaultValue() function that we can trivially modify to start adding undefs again, perhaps protected by a command line argument, if that ended up being desirable. I'll make that change if it would benefit you. (But I still strongly believe that decreasing the number of undefs or at least leaving it unchanged is the correct default for basically any standard use case for llvm-reduce.)

hi Matt, I wanted first to echo what Nuno said and second say that if this change is somehow detrimental to performing test case reductions that you care about, please send me the specific example (or maybe just post it on discourse and tag me) and let's figure out what to do about it. My hope and belief here are that this probably won't happen, but you never know.

clang-format and address review

oof, sorry, I'm still figuring out the new interface, this is committed here:

add FIXME suggested by @aeubanks

this is very cool, Florian. do you mind letting us know (or maintaining a list somewhere) of bugs this finds?

Did you mean to check something like the following?

define i32 @src(i1 %cond, i32 %x) {
  %x2 = freeze i32 %x
  %s = select i1 %cond, i32 %x2, i32 undef
  ret i32 %s
}

define i32 @tgt(i1 %cond, i32 %x) {
  %x2 = freeze i32 %x
  ret i32 %x2
}

@craig.topper ok, I agree that should work. alive doesn't like it -- is this an alive bug @nlopes? a freeze should not yield undef.
https://alive2.llvm.org/ce/z/mWAsYv

@majnemer should work: https://alive2.llvm.org/ce/z/vL4yn4

Sorry but I don't think this can land until it has options for sending sanitizer output to Slack channels and SMS numbers.

so this works but seems heavy-handed:
http://volta.cs.utah.edu:8080/z/w-L7Jw

This seems good to me.

I'll just add that we (my students and I) are interested in making the UB semantics easily pluggable / switchable. They'll still (almost certainly) be in C++, but we'd like to factor these out so at least they're cleanly separated and easily swapped out.

I am also in the "be a bit conservative at first and see how things shake out" camp, but it's y'all doing the hard work here, not me :)

In D63423#1547216, @xbolva00 wrote:

The only remaining question is, as your said, whether or not to diagnose xors in macros. @regerh @jfb ?

Just wanted to say that I think I agree with the design choices here!