User Details
- User Since
- Sep 23 2015, 2:21 AM (390 w, 5 d)
Jan 4 2023
LGTM. I agree this is a good change, delta passes shouldn't try to be too clever.
Jan 3 2023
(I actually wanted the "Don't require any other arguments for --print-delta-passes" just the other day)
Jan 2 2023
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
Dec 1 2022
LGTM!
Nov 29 2022
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.
Oct 21 2022
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.
Oct 10 2022
LGTM
Oct 9 2022
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.
Oct 1 2022
Aug 15 2022
I like this -- especially the part that gets rid of the interestingness test's output
Aug 13 2022
Aug 10 2022
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
Aug 4 2022
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.
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
Aug 3 2022
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!
address comment by Matt
Aug 2 2022
thanks Arthur-- I've added this test and also refined the safety check in the pass to be less conservative
Jul 13 2022
I definitely support renaming any inaccurately named tests to be more accurate
Jun 27 2022
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.
Jun 23 2022
Jun 22 2022
clang-format and address review
Jun 21 2022
Jan 10 2022
oof, sorry, I'm still figuring out the new interface, this is committed here:
add FIXME suggested by @aeubanks
Jan 8 2022
Feb 14 2021
this is very cool, Florian. do you mind letting us know (or maintaining a list somewhere) of bugs this finds?
Jul 8 2020
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
Apr 1 2020
Sorry but I don't think this can land until it has options for sending sanitizer output to Slack channels and SMS numbers.
Mar 14 2020
so this works but seems heavy-handed:
http://volta.cs.utah.edu:8080/z/w-L7Jw
Feb 26 2020
This seems good to me.
Feb 10 2020
Oct 4 2019
Jul 16 2019
Jul 7 2019
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.
Jun 19 2019
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 :)
Jun 17 2019
Just wanted to say that I think I agree with the design choices here!