-
Notifications
You must be signed in to change notification settings - Fork 4
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
pre-commit: test PR101049 #1194
Conversation
baseline: llvm/llvm-project@80c5ccd
aa1efe3 pre-commit: Update |
115: ; preds = %"_ZN71_$LT$core..option..Option$LT$T$GT$$u20$as$u20$core..cmp..PartialOrd$GT$11partial_cmp17hd2b066d5f3f3306aE.exit", %97 | ||
%.041 = phi ptr [ %26, %97 ], [ %spec.select, %"_ZN71_$LT$core..option..Option$LT$T$GT$$u20$as$u20$core..cmp..PartialOrd$GT$11partial_cmp17hd2b066d5f3f3306aE.exit" ] | ||
%.0.i45 = phi i8 [ %.0.i.i, %111 ], [ %..i, %108 ] | ||
%112 = icmp eq i8 %.0.i45, 1 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not exactly a regression (previous IR looks worse?) but there is an optimization opportunity here: If we fold the icmp into the phi then the ucmp becomes an icmp and the other one folds to the zext argument.
%28 = icmp ult i64 %13, %17 | ||
%.neg.i.i.i = sext i1 %28 to i32 | ||
%29 = add nsw i32 %.neg.i.i.i, %27 | ||
%30 = icmp slt i32 %29, 0 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not sure why this regresses, but it would get fixed by llvm/llvm-project#103833 anyway.
%136 = getelementptr inbounds i8, ptr %117, i64 56 | ||
%.0.i.i.i.i = call i8 @llvm.ucmp.i8.i64(i64 %126, i64 %123) | ||
%.0.i.i50 = select i1 %129, i8 %.0.i.i.i.i, i8 %127 | ||
%130 = icmp eq i8 %.0.i.i50, -1 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Regression.
switch i8 %.0.i12.i, label %_ZN3mio4poll8Registry10deregister17h8db01216e80c6d9cE.exit [ | ||
i8 0, label %.critedge11.i | ||
i8 -1, label %.critedge11.i | ||
] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Regression.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this one would be handled by adding ConstantRange support for ucmp, so we see the result can only be 0 or -1 and eliminate the default case.
296115f
to
7225cd8
Compare
PR Link: llvm/llvm-project#101049