[clang-tools-extra] Clean up strange uses of getAnalysisIfAvailable (PR #65729)

[Björn Pettersson via cfe-commits]

bjope wrote:

> > I guess I don't know how pull requests and reviewing works in github. I 
> > actually added 3 comments on this patch a several days (or weeks) ago. But 
> > turns out that they were "pending" because I had only "started review" and 
> > not found the place to "submit review".
> 
> For that reason I usually click "add single comment" instead of "start a 
> review".

Thanks. I'll try to remember that.

If they for example had named the buttons "Post comment directly" and "Save as 
draft", then it has been more obvious.
I also do not know how to find all my pending "draft reviews" (unless I'm 
already "assigned" or "requested as reviewer") as the https://github.com/pulls 
is just crap compared to my old dashboard in phabricator.

https://github.com/llvm/llvm-project/pull/65729
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang-tools-extra] Clean up strange uses of getAnalysisIfAvailable (PR #65729)

[Jay Foad via cfe-commits]

@@ -935,7 +935,7 @@ bool 
AArch64ConditionalCompares::runOnMachineFunction(MachineFunction &MF) {
   SchedModel = MF.getSubtarget().getSchedModel();
   MRI = &MF.getRegInfo();
   DomTree = &getAnalysis();
-  Loops = getAnalysisIfAvailable();
+  Loops = &getAnalysis();

jayfoad wrote:

Everything seems to work if I change `AArch64ConditionalCompares` to not 
require `MachineLoopInfo`. But I think I'll leave it to the AArch64 maintainers 
like you suggest.

https://github.com/llvm/llvm-project/pull/65729
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang-tools-extra] Clean up strange uses of getAnalysisIfAvailable (PR #65729)

[Jay Foad via cfe-commits]

@@ -261,7 +261,7 @@ bool VirtRegRewriter::runOnMachineFunction(MachineFunction 
&fn) {
   Indexes = &getAnalysis();
   LIS = &getAnalysis();
   VRM = &getAnalysis();
-  DebugVars = getAnalysisIfAvailable();
+  DebugVars = &getAnalysis();

jayfoad wrote:

Thanks! Fixed in 05c16f40c9de93c181d45ec718de5487380f0514

https://github.com/llvm/llvm-project/pull/65729
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang-tools-extra] Clean up strange uses of getAnalysisIfAvailable (PR #65729)

[Jay Foad via cfe-commits]

@@ -3280,7 +3280,7 @@ class llvm::gvn::GVNLegacyPass : public FunctionPass {
 if (skipFunction(F))
   return false;
 
-auto *LIWP = getAnalysisIfAvailable();
+auto &LIWP = getAnalysis();

jayfoad wrote:

I prefer not to leave it as is - I think either we should get the analysis with 
`getAnalysis` or we should remove the `addRequired`, so that they are 
consistent with each other.

https://github.com/llvm/llvm-project/pull/65729
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang-tools-extra] Clean up strange uses of getAnalysisIfAvailable (PR #65729)

[Björn Pettersson via cfe-commits]

@@ -3280,7 +3280,7 @@ class llvm::gvn::GVNLegacyPass : public FunctionPass {
 if (skipFunction(F))
   return false;
 
-auto *LIWP = getAnalysisIfAvailable();
+auto &LIWP = getAnalysis();

bjope wrote:

IIUC GVN doesn't depend on LoopInfo itself. It should just make sure to 
preserve it if it is available. So one might wonder if it is the addRequire 
that should be questioned and possibly removed instead.
But it's been like this for years (that it is required), and messing around 
with the legacy PM analysis dependencies for an IR pass (even if it is used by 
some backends) is perhaps not worth it.

Therefore I think you proposed change is ok. Or you could just leave this as 
is, since the IfAvailable part in some sense indicate that the pass itself 
doesn't depend on the analysis.

https://github.com/llvm/llvm-project/pull/65729
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang-tools-extra] Clean up strange uses of getAnalysisIfAvailable (PR #65729)

[Björn Pettersson via cfe-commits]

https://github.com/bjope commented:

I guess I don't know how pull requests and reviewing works in github. I 
actually added 3 comments on this patch a several days (or weeks) ago. But 
turns out that they were "pending" because I had only "started review" and not 
found the place to "submit review".

So sorry for some late comments here. Since this has been pushed already, then 
I guess you may ignore them. 

https://github.com/llvm/llvm-project/pull/65729
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang-tools-extra] Clean up strange uses of getAnalysisIfAvailable (PR #65729)

[Björn Pettersson via cfe-commits]

https://github.com/bjope edited https://github.com/llvm/llvm-project/pull/65729
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang-tools-extra] Clean up strange uses of getAnalysisIfAvailable (PR #65729)

[Jay Foad via cfe-commits]

https://github.com/jayfoad closed 
https://github.com/llvm/llvm-project/pull/65729
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits