[flang][NFC] AliasAnalysis: Use Indirect not Unknown for LoadOp

[jdenny-ornl]

As mentioned at #126156 (comment): PR #126156 causes AliasAnalysis::getSource to sometimes return SourceKind::Unknown when it used to return SourceKind::Indirect for a LoadOp. This patch restores that part of the old behavior. It should not affect user-visible behavior because AliasAnalysis::alias treats SourceKind::Unknown and SourceKind::Indirect equivalently, but it does improve debugging output.

[llvmbot]

@llvm/pr-subscribers-flang-fir-hlfir

Author: Joel E. Denny (jdenny-ornl)

Changes

As mentioned at
<#126156 (comment)>: PR #126156 causes AliasAnalysis::getSource to sometimes return SourceKind::Unknown when it used to return SourceKind::Indirect for a LoadOp. This patch restores that part of the old behavior. It should not affect user-visible behavior because AliasAnalysis::alias treats SourceKind::Unknown and SourceKind::Indirect equivalently, but it does improve debugging output.

---

Full diff: https://github.com/llvm/llvm-project/pull/127845.diff

1 Files Affected:

• (modified) flang/lib/Optimizer/Analysis/AliasAnalysis.cpp (+5-6)

diff --git a/flang/lib/Optimizer/Analysis/AliasAnalysis.cpp b/flang/lib/Optimizer/Analysis/AliasAnalysis.cpp
index 70fa18ad65b9b..d881b630d33d9 100644
--- a/flang/lib/Optimizer/Analysis/AliasAnalysis.cpp
+++ b/flang/lib/Optimizer/Analysis/AliasAnalysis.cpp
@@ -641,14 +641,13 @@ AliasAnalysis::Source AliasAnalysis::getSource(mlir::Value v,
             } else if (isDummyArgument(def)) {
               defOp = nullptr;
               v = def;
+            } else {
+              type = SourceKind::Indirect;
             }
-
-            breakFromLoop = true;
-            return;
+          } else {
+            // No further tracking for addresses loaded from memory for now.
+            type = SourceKind::Indirect;
           }
-
-          // No further tracking for addresses loaded from memory for now.
-          type = SourceKind::Indirect;
           breakFromLoop = true;
         })
         .Case<fir::AddrOfOp>([&](auto op) {

[jdenny-ornl]

@Renaud-K If you're about to land a patch to rewrite all this and fix this issue anyway, feel free to ignore this PR. Otherwise, I thought it could be helpful in the meantime.

[Renaud-K]

Thank you for the fix.

[jdenny-ornl]

Thanks.

Do you know why the windows bot is failing to build? It reports:

[5290/5897] Building CXX object tools\flang\lib\Optimizer\Analysis\CMakeFiles\FIRAnalysis.dir\AliasAnalysis.cpp.obj
FAILED: tools/flang/lib/Optimizer/Analysis/CMakeFiles/FIRAnalysis.dir/AliasAnalysis.cpp.obj
sccache C:\BuildTools\VC\Tools\MSVC\14.29.30133\bin\Hostx64\x64\cl.exe  /nologo /TP -DFLANG_INCLUDE_TESTS=1 -DFLANG_LITTLE_ENDIAN=1 -DGTEST_HAS_RTTI=0 -DUNICODE -D_CRT_NONSTDC_NO_DEPRECATE -D_CRT_NONSTDC_NO_WARNINGS -D_CRT_SECURE_NO_DEPRECATE -D_CRT_SECURE_NO_WARNINGS -D_GLIBCXX_ASSERTIONS -D_HAS_EXCEPTIONS=0 -D_SCL_SECURE_NO_DEPRECATE -D_SCL_SECURE_NO_WARNINGS -D_UNICODE -D__STDC_CONSTANT_MACROS -D__STDC_FORMAT_MACROS -D__STDC_LIMIT_MACROS -Itools\flang\lib\Optimizer\Analysis -IC:\ws\src\flang\lib\Optimizer\Analysis -IC:\ws\src\flang\include -Itools\flang\include -Iinclude -IC:\ws\src\llvm\include -IC:\ws\src\flang\..\mlir\include -Itools\mlir\include -Itools\clang\include -IC:\ws\src\llvm\..\clang\include /DWIN32 /D_WINDOWS   /Zc:inline /Zc:preprocessor /Zc:__cplusplus /Oi /bigobj /permissive- /W4 -wd4141 -wd4146 -wd4244 -wd4267 -wd4291 -wd4351 -wd4456 -wd4457 -wd4458 -wd4459 -wd4503 -wd4624 -wd4722 -wd4100 -wd4127 -wd4512 -wd4505 -wd4610 -wd4510 -wd4702 -wd4245 -wd4706 -wd4310 -wd4701 -wd4703 -wd4389 -wd4611 -wd4805 -wd4204 -wd4577 -wd4091 -wd4592 -wd4319 -wd4709 -wd5105 -wd4324 -wd4251 -wd4275 -w14062 -we4238 /Gw /O2 /Ob2  -MD  /EHs-c- /GR- -UNDEBUG -std:c++17 /showIncludes /Fotools\flang\lib\Optimizer\Analysis\CMakeFiles\FIRAnalysis.dir\AliasAnalysis.cpp.obj /Fdtools\flang\lib\Optimizer\Analysis\CMakeFiles\FIRAnalysis.dir\FIRAnalysis.pdb /FS -c C:\ws\src\flang\lib\Optimizer\Analysis\AliasAnalysis.cpp
C:\ws\src\flang\lib\Optimizer\Analysis\AliasAnalysis.cpp(651): error C2065: 'breakFromLoop': undeclared identifier
C:\ws\src\llvm\include\llvm/ADT/TypeSwitch.h(149): note: see reference to function template instantiation 'void fir::AliasAnalysis::getSource::<lambda_e5542b942d65461609ca2a2290f9cd06>::operator ()<To>(To) const' being compiled
        with
        [
            To=fir::LoadOp
        ]
C:\ws\src\flang\lib\Optimizer\Analysis\AliasAnalysis.cpp(564): note: see reference to function template instantiation 'llvm::TypeSwitch<mlir::Operation *,void> &llvm::TypeSwitch<mlir::Operation *,void>::Case<fir::LoadOp,fir::AliasAnalysis::getSource::<lambda_e5542b942d65461609ca2a2290f9cd06>>(CallableT &&)' being compiled
        with
        [
            CallableT=fir::AliasAnalysis::getSource::<lambda_e5542b942d65461609ca2a2290f9cd06>
        ]
C:\ws\src\flang\lib\Optimizer\Analysis\AliasAnalysis.cpp(771): note: see reference to function template instantiation 'llvm::TypeSwitch<mlir::Operation *,void> &llvm::TypeSwitch<mlir::Operation *,void>::Case<fir::LoadOp,fir::AliasAnalysis::getSource::<lambda_e5542b942d65461609ca2a2290f9cd06>>(CallableT &&)' being compiled
        with
        [
            CallableT=fir::AliasAnalysis::getSource::<lambda_e5542b942d65461609ca2a2290f9cd06>
        ]

[Renaud-K]

hard to tell without a windows compiler which I am not setup to run. It is like it is missing a curly brace or something. But I don't think we need the second else clause. We can let the code fall through this part. Of course we would have to return in the then clause.

Basically I would troubleshoot with smaller diffs.

[jdenny-ornl]

I think you're suggesting to remove else { and } but keep the type = SourceKind::Indirect; that appears within it. But that would overwrite the type set previously for AllocaOp and AddrOfOp.

[jdenny-ornl]

Sorry, I replied above before I saw your comment's edit.

I'm not set up for windows compilation either. I'm surprised that different compilers have different ideas about whether breakFromLoop is declared. I'll try to figure this out tomorrow.

[jdenny-ornl]

I found a small workaround for the windows pre-commit CI build failure (draft PR #128025 has the investigation), and I've just pushed it to this PR. @Renaud-K, does it look ok to you?

I'll also ask in LLVM discord #windows for help deciding if this is an msvc bug that I should report.

[github-actions]

✅ With the latest revision this PR passed the C/C++ code formatter.