emit vector info of traceback table.
Details
Diff Detail
- Repository
- rG LLVM Github Monorepo
Time | Test | |
---|---|---|
50 ms | x64 debian > LLVM.CodeGen/PowerPC::aix-emit-tracebacktable-vectorinfo_hasvarg.ll Script:
--
: 'RUN: at line 7'; not --crash /mnt/disks/ssd0/agent/llvm-project/build/bin/llc -verify-machineinstrs -mtriple powerpc-ibm-aix-xcoff -mcpu=pwr7 -mattr=+altivec -vec-extabi -xcoff-traceback-table=true 2>&1 < /mnt/disks/ssd0/agent/llvm-project/llvm/test/CodeGen/PowerPC/aix-emit-tracebacktable-vectorinfo_hasvarg.ll | /mnt/disks/ssd0/agent/llvm-project/build/bin/FileCheck --allow-unused-prefixes=false --check-prefixes=CHECK-ASM,COMMON /mnt/disks/ssd0/agent/llvm-project/llvm/test/CodeGen/PowerPC/aix-emit-tracebacktable-vectorinfo_hasvarg.ll
| |
70 ms | x64 windows > LLVM.CodeGen/PowerPC::aix-emit-tracebacktable-vectorinfo_hasvarg.ll Script:
--
: 'RUN: at line 7'; not --crash c:\ws\w64\llvm-project\premerge-checks\build\bin\llc.exe -verify-machineinstrs -mtriple powerpc-ibm-aix-xcoff -mcpu=pwr7 -mattr=+altivec -vec-extabi -xcoff-traceback-table=true 2>&1 < C:\ws\w64\llvm-project\premerge-checks\llvm\test\CodeGen\PowerPC\aix-emit-tracebacktable-vectorinfo_hasvarg.ll | c:\ws\w64\llvm-project\premerge-checks\build\bin\filecheck.exe --allow-unused-prefixes=false --check-prefixes=CHECK-ASM,COMMON C:\ws\w64\llvm-project\premerge-checks\llvm\test\CodeGen\PowerPC\aix-emit-tracebacktable-vectorinfo_hasvarg.ll
|
Event Timeline
llvm/lib/BinaryFormat/XCOFF.cpp | ||
---|---|---|
142 | Something is wrong if Value != 0u, but I think it's normal when we have more parameters than the uint32 type could encode. And in those situation, as Hubert suggested, we could print ... to represent any values that we could not decode. Please have a test case for this case as well. | |
llvm/lib/Object/XCOFFObjectFile.cpp | ||
852 | This Error as an parameter in the constructor is too easy to get ignored. If we need to do this, we would do the whole set which includes making this constructor private, and have a Create function to return an error or the object, which is described here: https://llvm.org/docs/ProgrammersManual.html#fallible-constructors | |
llvm/lib/Target/PowerPC/PPCAsmPrinter.cpp | ||
2054 | This .get() won't work if you are compiling in a non-assert mode. | |
llvm/lib/Target/PowerPC/PPCMachineFunctionInfo.cpp | ||
70 | Please fix all clang-tidy warnings. | |
70 | Instead of traversing this array for 3 times (getFixedParmsNum, getFloatingPointParmsNum, getVectorParmsNum), why not just still increment the corresponding data member in appendParameterType()? | |
73 | 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. | |
76 | You could pull this out of the every case, and put it just after the switch statement. As you are doing it for every case. | |
97 | An assert here would be good. |
llvm/lib/Target/PowerPC/PPCMachineFunctionInfo.cpp | ||
---|---|---|
70 | I think we only use here for getFloatingPointParmsNum, getVectorParmsNum once. | |
73 | it can not pull out, for there are other parameter type (for example FixedType), we do not need deal with in the function. | |
97 | I can not add assert here for there are other parameter type (for example FixedType). |
llvm/include/llvm/Object/XCOFFObjectFile.h | ||
---|---|---|
398–399 | Let's remove this "friend" so that no one could call the constructor directly. | |
llvm/lib/BinaryFormat/XCOFF.cpp | ||
229–230 | minor style nit. | |
255 | default assert here? | |
llvm/lib/Object/XCOFFObjectFile.cpp | ||
923–924 | ||
960 | I thought we only need to do this ErrorAsOutParameter once at the beginning of the function. Why do we need to do it 3 times at here and 981 and 992? | |
llvm/lib/Target/PowerPC/PPCAsmPrinter.cpp | ||
1968 | Only checking error in assert won't work in non-assertion build. | |
2017 | I think we are only supposed to set this bit when the special register VRSAVE is saved on stack. | |
2030 | There is also a VSCR register, do we need to check that? Or are we inferring checking V0 to V31 would be enough to find out if any vmx instruction is used? If so, a comment would be desired. | |
llvm/lib/Target/PowerPC/PPCMachineFunctionInfo.cpp | ||
69 | What if we don't have parameters that are vectors, but we use vectors inside of the function? I thought we should set HasVectorInfo in those situation as well. | |
70 | It's possible these functions got called later for other purposes and we don't know. | |
75 | ||
183 | Address the Lint comment. | |
llvm/test/CodeGen/PowerPC/aix-emit-tracebacktable-vectorinfo.ll | ||
57 | Is it possible to have tests for these fields? |
llvm/include/llvm/Object/XCOFFObjectFile.h | ||
---|---|---|
398–399 | thanks | |
llvm/lib/BinaryFormat/XCOFF.cpp | ||
255 | there 4 cases for 2 bits, I do not think we need default here. | |
llvm/lib/Object/XCOFFObjectFile.cpp | ||
923–924 | thanks | |
llvm/lib/Target/PowerPC/PPCAsmPrinter.cpp | ||
1968 | thanks | |
2017 | Hubert point out that XL sets the bit (at least when there are other vector registers saved) , maybe we need double confirm with hubert @hubert.reinterpretcast | |
2030 | I do not think we need to check the VSCR. for "Special instructions | |
llvm/lib/Target/PowerPC/PPCMachineFunctionInfo.cpp | ||
69 | thanks for point out. | |
llvm/test/CodeGen/PowerPC/aix-emit-tracebacktable-vectorinfo.ll | ||
57 | I can not add a test case for it. There is error as " error in backend: variadic arguments for vector types are unimplemented for AIX" |
llvm/lib/Target/PowerPC/PPCAsmPrinter.cpp | ||
---|---|---|
2017 | You're both right. XL sets the bit (I don't think it was supposed to). The funny thing is that setting the bit prevents a possible misinterpretation of the traceback table by the XL C++ runtime because of a confluence of errors. |
llvm/lib/Target/PowerPC/PPCAsmPrinter.cpp | ||
---|---|---|
1870 | I have concerns about writing our own version of this function. | |
2016 | ||
llvm/test/CodeGen/PowerPC/aix-emit-tracebacktable-vectorinfo.ll | ||
57 | Sure. I saw you add a test case to test the error for HasVarArgs, which is good. | |
llvm/test/CodeGen/PowerPC/aix-emit-tracebacktable-vectorinfo_hasvarg.ll | ||
6 | I don't think we want to use include in the lit test. You could write the declaration yourself. And you probably don't even need to call vec_abs if you just want to check if HasVarArgs bit is set? |