[PATCH] D93659: [AIX][XCOFF] emit vector info of traceback table.

Digger via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Mon Jan 4 13:18:58 PST 2021


DiggerLin marked 8 inline comments as done.
DiggerLin added inline comments.


================
Comment at: llvm/lib/Target/PowerPC/PPCMachineFunctionInfo.cpp:70
+  unsigned Num = 0;
+  for (auto &Elt : ParamtersType)
+    if (Elt == FixedType)
----------------
jasonliu wrote:
> jasonliu wrote:
> > Please fix all clang-tidy warnings.
> Instead of traversing this array for 3 times (getFixedParmsNum, getFloatingPointParmsNum, getVectorParmsNum), why not just still increment the corresponding data member in appendParameterType()?
I think we only use here for getFloatingPointParmsNum, getVectorParmsNum once.
I can increment corresponding data member , but it need three new data member here.


================
Comment at: llvm/lib/Target/PowerPC/PPCMachineFunctionInfo.cpp:107
+    case VectorChar:
+      VectExtParamInfo <<= XCOFF::TracebackTable::WidthOfParamType;
+      VectExtParamInfo |=
----------------
jasonliu wrote:
> You could pull this out of the every case, and put it just before the switch statement. As you are doing it for every case.
it can not pull out, for there are other parameter type (for example FixedType), we do not need deal with in the function.


================
Comment at: llvm/lib/Target/PowerPC/PPCMachineFunctionInfo.cpp:131
+    default:
+      break;
+    }
----------------
jasonliu wrote:
> An assert here would be good.
I can not add assert here for there are other parameter type (for example FixedType).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D93659



More information about the llvm-commits mailing list