[PATCH] D92398: [AIX][XCOFF] emit traceback table for function in aix

[Hubert Tong via Phabricator via llvm-commits]

hubert.reinterpretcast added inline comments.


================
Comment at: llvm/lib/Target/PowerPC/PPCMachineFunctionInfo.cpp:67
+
+void PPCFunctionInfo::setParameterType(ParamType Type, unsigned RegNo) {
+  unsigned CopyParamType = ParameterType;
----------------
Is `RegNo` a correct name for this parameter? It's not used like a register number (e.g., `PPC::R3`).


================
Comment at: llvm/lib/Target/PowerPC/PPCMachineFunctionInfo.cpp:68
+void PPCFunctionInfo::setParameterType(ParamType Type, unsigned RegNo) {
+  unsigned CopyParamType = ParameterType;
+  unsigned Bits = 0;
----------------



================
Comment at: llvm/lib/Target/PowerPC/PPCMachineFunctionInfo.cpp:70
+  unsigned Bits = 0;
+  for (unsigned I = 0; I < RegNo; I++) {
+    if (CopyParamType & 0x80000000) {
----------------
Prefix `++` please.


================
Comment at: llvm/lib/Target/PowerPC/PPCMachineFunctionInfo.cpp:71
+  for (unsigned I = 0; I < RegNo; I++) {
+    if (CopyParamType & 0x80000000) {
+      // '10'b => floating point short parameter.
----------------
This is `ParmTypeIsFloatingBit`?


================
Comment at: llvm/lib/Target/PowerPC/PPCMachineFunctionInfo.cpp:79
+      CopyParamType <<= 1;
+      Bits++;
+    }
----------------
Same comment re: `++`.


================
Comment at: llvm/lib/Target/PowerPC/PPCMachineFunctionInfo.cpp:83-86
+  if (Type == FixedType)
+    ParameterType |= Type << (31 - Bits);
+  else
+    ParameterType |= Type << (30 - Bits);
----------------
There can be 8 GPRs of arguments followed by 13 FPRs of arguments. 8 + 13 * 2 = 34. Shifting by a negative number of bits is undefined behaviour.


================
Comment at: llvm/test/CodeGen/PowerPC/aix-emit-tracebacktable.ll:16
+
+define double @_Z10add_structifd1SP2SD1Di(i32 %value, float %fvalue, double %dvalue, %struct.S* byval(%struct.S) align 4 %s, %struct.SD* %dp, %struct.D* byval(%struct.D) align 4 %0, i32 %v2) #0 {
+entry:
----------------
Please add a test where a struct taking more than one register occurs prior to a float.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D92398