[Mlir-commits] [mlir] Fix loop_annotation name in unit tests (PR #172934)

[Mehdi Amini]

joker-eph wrote:

I think we have a more fundamental problem here where this all works "by accident".

The `llvm.` prefix for discardable attributes is in theory necessary: discardable attributes should have a dialect prefix (we just never wrote the verifier for this :( ).

However the `llvm.br` operation has a **inherent** attribute named `loop_annotation`.

It turns out that we copy LLVM dialect attributes when lowering SCF to CF:

```
static void propagateLoopAttrs(Operation *scfOp, Operation *brOp) {
  // Let the CondBranchOp carry the LLVM attributes from the ForOp, such as the
  // llvm.loop_annotation attribute.
  // LLVM requires the loop metadata to be attached on the "latch" block. Which
  // is the back-edge to the header block (conditionBlock)
  SmallVector<NamedAttribute> llvmAttrs;
  llvm::copy_if(scfOp->getAttrs(), std::back_inserter(llvmAttrs),
                [](auto attr) {
                  return isa<LLVM::LLVMDialect>(attr.getValue().getDialect());
                });
  brOp->setDiscardableAttrs(llvmAttrs);
}
```

However here we check just for the dialect of the attribute (value) and not the key, so we propagate a discardable attribute whose key just collide with the inherent attribute of the LLVM branch.
This is all broken :) 


https://github.com/llvm/llvm-project/pull/172934