[PATCH] D42468: [lldb][PPC64] Fixed vector and struct return value

[Leandro Lupori via Phabricator via llvm-commits]

luporl added inline comments.


================
Comment at: source/Plugins/ABI/SysV-ppc64/ABISysV_ppc64.cpp:393-421
+#define LOG_PREFIX "ReturnValueExtractor: "
 
-  RegisterContext *reg_ctx = thread.GetRegisterContext().get();
-  if (!reg_ctx)
-    return return_valobj_sp;
+#define LOG(...) \
+  m_logger.LogMsg(LOG_PREFIX __VA_ARGS__)
 
-  const uint32_t type_flags = return_compiler_type.GetTypeInfo();
-  if (type_flags & eTypeIsScalar) {
-    value.SetValueType(Value::eValueTypeScalar);
+class ReturnValueExtractor {
+  // Log wrapper
----------------
labath wrote:
> The `LLDB_LOG` macro might be of interest to you. It will not be *as* concise as your solution here, but it should get you most of the benefits without having to reinvent your own logging infrastructure.
Ok, thanks, I'll take a look at it.


================
Comment at: source/Plugins/ABI/SysV-ppc64/ABISysV_ppc64.cpp:551-579
+  // constructor - can't fail
+  ReturnValueExtractor(Thread &thread, CompilerType &type)
+    : m_thread(thread),
+      m_type(type) {
+    m_byte_size = m_type.GetByteSize(nullptr);
+    m_data_ap.reset(new DataBufferHeap(m_byte_size, 0));
+  }
----------------
labath wrote:
> Instead of the constructor+init combo you can use a static factory function which returns `llvm::Expected<ReturnValueExtractor>`. The factory function can do all the operations that can fail, and then construct the object using a simple (possibly private) constructor. That way you never end up having a partially initialized object.
Ok, I'll make this change.


================
Comment at: source/Plugins/ABI/SysV-ppc64/ABISysV_ppc64.cpp:709
+      case sizeof(float):
+        value_sp->GetScalar() = (float)de.GetDouble(&offset);
         break;
----------------
labath wrote:
> Should this really be GetDouble?
Yes, it must really be GetDouble.
It seems that PowerPC64 always stores floating point values as doubles in its FP registers, even float ones.
The previous code, that used GetFloat, was not working properly.


Repository:
  rL LLVM

https://reviews.llvm.org/D42468