[PATCH] D77874: [AArch32] Armv8.6a Matrix Mul Assembly

Luke Geeson via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Mon Apr 20 08:39:26 PDT 2020


LukeGeeson marked an inline comment as done.
LukeGeeson added inline comments.


================
Comment at: llvm/lib/Target/ARM/AsmParser/ARMAsmParser.cpp:6336
+      Mnemonic == "vusmmla" || Mnemonic == "vsudot" ||
+      Mnemonic == "vusdot")
     return Mnemonic;
----------------
simon_tatham wrote:
> Now I look closely, I'm slightly confused about why all of these instructions needed to be added to this if statement. I thought this if statement was supposed to be a list of exceptions to the general rule that a mnemonic ending in a known condition-code string like "eq" or "ne" or "lt" should be broken up into a shorter mnemonic and a conditional suffix. Hence, for example, "wls" is on the list so that it doesn't get misparsed as "w" with condition "ls".
> 
> Mind you, I'm also slightly confused about what some of the existing mnemonics in this list are here for. But what specific thing fails if you remove this hunk of the diff?
Nothing fails when removing this hunk and running tests (check-all, llvm/test, clang/test).

I agree with you in principle, but I suppose here I'm appealing to the age old adage of 'we did it this way here' 

I think we should keep it, and log a separate issue for refactoring this statement in a different issue


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77874





More information about the llvm-commits mailing list