[PATCH] D137832: [AMDGPU][AsmParser] Forbid TFE modifiers for MBUF stores.

[Ivan Kosarev via Phabricator via llvm-commits]

kosarev added a comment.

> Could you add a test to check the reported error position? Unfortunately, AsmMatcher often fails to report the location of offending operand. Moving the check to validator may improve diagnostic by using `getImmLoc`.

The currently reported position is the beginning of the first operand and in the current design there seems to be no way to do better, which looks a somewhat severe problem to me and I'd be happy to improve it, but that feels well beyond the scope of this patch, doesn't it.



================
Comment at: llvm/lib/Target/AMDGPU/AsmParser/AMDGPUAsmParser.cpp:3282
+    for (const auto &Op : Operands) {
+      if (static_cast<const AMDGPUOperand &>(*Op).isTFE())
+        return Match_InvalidOperand;
----------------
Joe_Nash wrote:
> foad wrote:
> > Joe_Nash wrote:
> > > I would prefer dynamic_cast, for safe downcasting.
> > LLVM is generally built with -fno-rtti so I don't think you can use dynamic_cast (can you?).
> Good point, I don't know if it's available. LLVM dyn_cast may also be an option.
I understand `dyn_cast<>` is normally used where we have to discriminate between several possibilities with `static_cast<>` being the standard solution when we know the dynamic type.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D137832/new/

https://reviews.llvm.org/D137832