- User Since
- Aug 18 2016, 4:39 AM (126 w, 6 d)
The patch currently implements variant 1 form the paper (TDMSC-I) as it is slightly simpler. I will follow up with variant 2 (TDMSC-II) in a bit and do some more detailed compile time measurement. Also the only reason MergeSets live in IteratedDominanceFrontier.h/cpp was convenience, they could be moved to their own files.
Sounds good to me, I'll move the code to LCSSA without DT checks. Thanks!
Mon, Jan 21
Fri, Jan 18
Thu, Jan 17
state explicitly that sub-loops must be in LCSSA form already for formLCSSA()
Update places to use getMaxNumOperands, add more context to description.
updated to use SDNode::getMaxNumOperands
Wed, Jan 16
update to use getTokenFactor.
Thanks Eli, changed to getTokenFactor.
Tue, Jan 15
Committed in rL351187, thanks!
Mon, Jan 14
Thanks, added check for ConstantExpr.
Sat, Jan 12
LGTM, thanks! Please wait a bit with committing, in case anybody else has additional comments.
LGTM, thanks! It would be great if you could extend the test cases a bit more before committing, covering cases with shifted regs and cases where some SUs have multiple successors and not all of them should be fused.
Fri, Jan 11
Requesting changes as additional info on the bzip and PDF regressions is needed.
Jessica, are you still planing on pushing this change? :)
The AArch64 side LGTM. Also adding Oliver and Sjoerd as they worked on FP16 side as well, in case they have any thoughts.
Diego, are you still planning to land this patch?
Marking this as requiring changes, as the MachinePipeliner code is getting refactored a bit in D56084. Please let us know when you want us to review the patch again.
Would it be possible to have a MIR test here, which just runs the scheduler?
Wed, Jan 9
It might be good to mention those guidelines explicitly here https://llvm.org/docs/Contributing.html#how-to-submit-a-patch.
The bot is back to passing after reverting: http://green.lab.llvm.org/green/job/clang-stage1-configure-RA/52757/. Please let me know if you have trouble reproducing the failure.
One of the errors with TSan and this patch on macOS is the following while running binaries compiled with TSan:
It looks like this patch may have broken some bots on green dragon, e.g lots of ThreadSanitizer and UBSan-ThreadSanitizer-x86_64 tests stared failing after this went in: http://green.lab.llvm.org/green/job/clang-stage1-configure-RA/52725/
Tue, Jan 8
I am seeing lldb-cmake failing again, with the same error as with this patch earlier: http://green.lab.llvm.org/green/job/lldb-cmake/16557/
Mon, Jan 7
LGTM, thanks, but it would be good if someone more familiar with AMDGPU could also have a quick look.
Great thanks! IMO the caller of ShrinkDemandedConstant should make sure the demanded bits valid for all users, as the existing code in TargetLowering.cpp already should do.
I am seeing a regression with this change on AArch64, where we do not use shrinked constants, e.g. for and/or if they have multiple users. I am not entirely sure what the problem is this patch is solving, as I am not familiar with AMDGPU. Is it a case where shrinking the constant leads to shrinking the related Op and in the end this introduces unnecessary conversion code, as multiple users require different bitwidths?
Fri, Jan 4
Thanks! Do you need someone to commit the change on your behalf?
Thu, Jan 3
Thanks for the patch! I plan to have a closer look in the next few days.
Looks like the patch has just been reverted in rL350345. Please let me know if you have problems reproducing the failure.
It looks like this commit breaks the lldb-cmake bot again, with the following error (http://green.lab.llvm.org/green/job/lldb-cmake/16085/consoleFull#-15612593049ba4694-19c4-4d7e-bec5-911270d8a58c) :
Wed, Jan 2
Are there any instances in the current codebase where this would be useful?
Dec 21 2018
Dec 20 2018
Updated to just use Defs = [CPSR], as suggested by Eli.
Reduced patch to just add CPSR to live-ins of newly created blocks, if required.
Use LivePhysRegs for liveins computation, pushed code to ARMISelLowering, handled all cases I could spot there. Adding Eli, as he fixed a similar problem in D54192, which I think should also be handled by the general liveins computation.
Dec 19 2018
Rename to PossiblySafeWithRtChecks and FoundNonConstantDistanceDependence.