<html><head><meta http-equiv="Content-Type" content="text/html charset=us-ascii"></head><body style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space; ">Hey,<div><br></div><div>I have a serious question about this patch that I raised in a prior email, and I'd appreciate it if you'd address it rather than continuing in the opposite direction.</div><div><br><div><div>On Jun 5, 2013, at 12:42 PM, Matt Arsenault <<a href="mailto:Matthew.Arsenault@amd.com">Matthew.Arsenault@amd.com</a>> wrote:</div><br class="Apple-interchange-newline"><blockquote type="cite"> Move NearestCommonDominator to separate file<br><br><a href="http://llvm-reviews.chandlerc.com/D906">http://llvm-reviews.chandlerc.com/D906</a><br><br>CHANGE SINCE LAST DIFF<br> http://llvm-reviews.chandlerc.com/D906?vs=2252&id=2279#toc<br><br>Files:<br> include/llvm/Analysis/NearestCommonDominator.h<br></blockquote><div><br></div><div>I don't understand why this analysis is necessary. We already have pair-wise nearestCommonDominator functionality built into DomTree. If it needs to be extended to support multi-block NCD queries, that should be possible to implement on top of the existing functionality rather than build a new analysis on top of it.</div><br><blockquote type="cite"> include/llvm/InitializePasses.h<br> include/llvm/LinkAllPasses.h<br> include/llvm/Transforms/Scalar.h<br> lib/Target/R600/AMDGPUStructurizeCFG.cpp<br> lib/Target/R600/AMDGPUTargetMachine.cpp<br> lib/Target/R600/CMakeLists.txt<br> lib/Transforms/Scalar/CMakeLists.txt<br> lib/Transforms/Scalar/Scalar.cpp<br> lib/Transforms/Scalar/StructurizeCFG.cpp<br></blockquote><div><br></div></div>More generally, I don't understand why it's desirable to move this out of the R600 target. The comments hand wave about it being useful for other targets, but I don't see anyone actually planning to use it. If there is interest in it from other targets, then you probably need to <i>start</i> by having a conversation with other target authors about what kind of common canonical form different targets can share rather than simply foisting whatever was convenient for R600 onto them.</div><div><br></div><div>--Owen</div></body></html>