[mlir] Use getAttr to replace getInherentAttr in the test-symbol-uses pass

[linuxlonelyeagle]

Fix #172603.

[llvmbot]

@llvm/pr-subscribers-mlir

Author: lonely eagle (linuxlonelyeagle)

Changes

Fix #172603.

---

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

1 Files Affected:

• (modified) mlir/test/lib/IR/TestSymbolUses.cpp (+1-1)

diff --git a/mlir/test/lib/IR/TestSymbolUses.cpp b/mlir/test/lib/IR/TestSymbolUses.cpp
index b470b15c533b5..59014b93a0df7 100644
--- a/mlir/test/lib/IR/TestSymbolUses.cpp
+++ b/mlir/test/lib/IR/TestSymbolUses.cpp
@@ -59,7 +59,7 @@ struct SymbolUsesPass
               symbolUse.getUser()->getParentOp(), symbolUse.getSymbolRef())) {
         symbolUse.getUser()->emitRemark()
             << "found use of symbol : " << symbolUse.getSymbolRef() << " : "
-            << *symbol->getInherentAttr(SymbolTable::getSymbolAttrName());
+            << symbol->getAttr(SymbolTable::getSymbolAttrName());
       }
     }
     symbol->emitRemark() << "symbol has " << llvm::size(*symbolUses) << " uses";

[linuxlonelyeagle]

I didn't add anything new, I just fixed it by changing the use of the API. So I didn't add any new tests.

[joker-eph]

getAttr should be considered deprecated.
There is something suspicious here: which operation is "symbol" here?

[joker-eph]

Seems like the "launch_func" is the culprit, it does not seem to be defined as SymbolUserOpInterface, so I'm not even sure why it is visited at all. We seem to have some inconsistency here.

[joker-eph]

Ah no, this is necessarily an operation defining the SymbolOpInterface, can't be launch_func

[joker-eph]

So can we update:

WalkResult operateOnSymbol(Operation *symbol,

to be:

WalkResult operateOnSymbol(SymbolOpInterface symbol,

and rely on the interface instead?

[linuxlonelyeagle]

To be honest with you, I'm a bit confused. Using getAttr surprisingly works.I looked at the implementation of getAttr. Isn't it because getProperties Storage Size() returned 0? I thought dict-attr was not used when defining 'launch_func'. If there is anything wrong, I hope to point it out. I am sure there are some areas that I am not very clear about.😂

  Attribute getAttr(StringRef name) {
    if (getPropertiesStorageSize()) {
      if (std::optional<Attribute> inherentAttr = getInherentAttr(name))
        return *inherentAttr;
    }
    return attrs.get(name);
  }

[linuxlonelyeagle]

So can we update:

WalkResult operateOnSymbol(Operation *symbol,

to be:

WalkResult operateOnSymbol(SymbolOpInterface symbol,

and rely on the interface instead?

I think there is something a bit incorrect, I don't know if I misunderstood. Accessing "launch_func" is done through walk "gpu. func". The walk function does not directly access " launch_func ", In the crash example, it first walked "func.func" and then "gpu.func".

[linuxlonelyeagle]

Seems like the "launch_func" is the culprit, it does not seem to be defined as SymbolUserOpInterface, so I'm not even sure why it is visited at all. We seem to have some inconsistency here.

When walking to "gpu.func", the "symbol" (follow variable) is "gpu.func", we use SymbolTable::getSymbolUses(symbol, &module.getBodyRegion()); to visit the "launch_func"

    symbolUses = SymbolTable::getSymbolUses(symbol, &module.getBodyRegion());
    assert(symbolUses && "expected no unknown operations");
    for (SymbolTable::SymbolUse symbolUse : *symbolUses) {
      // Check that we can resolve back to our symbol.
      if (SymbolTable::lookupNearestSymbolFrom(
              symbolUse.getUser()->getParentOp(), symbolUse.getSymbolRef())) {
        symbolUse.getUser()->emitRemark()
            << "found use of symbol : " << symbolUse.getSymbolRef() << " : "
            << symbol->getAttr(SymbolTable::getSymbolAttrName());
      }
    }

[joker-eph]

I think there is something a bit incorrect, I don't know if I misunderstood. Accessing "launch_func" is done through walk "gpu. func". The walk function does not directly access " launch_func ", In the crash example, it first walked "func.func" and then "gpu.func".

I am not talking about launch_fun, the diff I sent above is unrelated to launch_func. It is about the method used to get the symbol ref and the use of the interface.