[PATCH] D81585: [AIX][XCOFF][Patch1] Provide decoding trace back table information API for xcoff object file for llvm-objdump -d

[Digger via Phabricator via llvm-commits]

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


================
Comment at: llvm/lib/Object/XCOFFObjectFile.cpp:898
+    // indicates the presence of vector parameters.
+    if (Cur && ParmNum > 0) {
+      uint32_t ParamsTypeValue = DE.getU32(Cur);
----------------
jhenderson wrote:
> Can you drop the `Cur &&` from here? It looks like there's no way it could be invalid at this point.
thanks


================
Comment at: llvm/unittests/Object/XCOFFObjectFileTest.cpp:150-159
+TEST(XCOFFObjectFileTest, XCOFFTracebackTableAPIHasVectorInfo) {
+  uint64_t Size = 40;
+  Expected<XCOFFTracebackTable> TTOrErr =
+      XCOFFTracebackTable::create(V_T, Size);
+  ASSERT_THAT_EXPECTED(TTOrErr, Succeeded());
+  XCOFFTracebackTable TT = *TTOrErr;
+
----------------
jhenderson wrote:
> DiggerLin wrote:
> > jhenderson wrote:
> > > The inputs to this test are identical to the previous case above. Could you simply merge the two together?
> > the two test case has different test case name, One to  test  XCOFFTracebackTableAPI for HasVectorInfo , Other to test XCOFFTracebackTableAPI for ControlledStorageInfoDisp.  If merge, it is difficult to have a good test case name here. 
> I guess when vector info parsing is expanded, this second test will get expanded, but the first won't be?
yes, the next patch will parse the VectorInfo expand info, the decode of parameters type  with VectorInfo  is true. 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D81585