[PATCH] D83111: [X86-64] Support Intel AMX Intrinsic

[Craig Topper via Phabricator via llvm-commits]

craig.topper added inline comments.


================
Comment at: clang/lib/Sema/SemaChecking.cpp:3613
+                                    ArrayRef<int> ArgNums,
+                                    int Low, int High) {
+  for (int ArgNum : ArgNums) {
----------------
xiangzhangllvm wrote:
> craig.topper wrote:
> > Why are Low/High arguments? Aren't they always 0 and 7.
> Yes, they are 0 and 7, keep the Low and High will make it easy to read.
Its sort of confusing to use a function arguments where the default value is always used. Would it be better to have clearly named static const global variables?


================
Comment at: clang/lib/Sema/SemaChecking.cpp:3630
+  // to represent the usage of them in bitset<8>.
+  std::bitset<8> ArgValues;
+  for (int ArgNum : ArgNums) {
----------------
Would probably make sense to have a named constant for this 8 here and the other one below.


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

https://reviews.llvm.org/D83111