[PATCH] Some code improvements (no functional change)

[Artyom Skrobov]

Hello,

It appears that Duncan no longer has the availability to review my three
suggested patches -- perhaps someone else could take a look?

Quoting below my follow-up mails, from two weeks ago, to Duncan's initial
comments on my patches:


-----Original Message-----
From: Artyom Skrobov [mailto:Artyom.Skrobov at arm.com] 
Sent: 17 April 2014 10:50
To: 'Duncan P. N. Exon Smith'
Cc: llvm-commits
Subject: RE: [PATCH] Some code improvements (no functional change)

Thank you Duncan.

> I don't think the code churn of the other include guards is worth it.
> They're private headers that work together as is, and haven't been
> modified since the original commit.

To be fair, regutils.h had been modified once since then (r81114); but is
there any reason not to add the guards?

> When the function is local to a file, SwapValue() is clear enough, but
> once it's API:  how do the names SwapValue() and SwapByteOrder() match
> up together?

I agree that SwapValue() is not a very good name to begin with.

Would it be reasonable if we name both SwapByteOrder() -- it's difficult to
describe their purpose in any other way -- and make the in-place function
take a pointer, instead of a reference?

That way, it would be more evident from the call site whether the function
operates on the argument or takes a copy.

> I think this one is definitely out, though.  The C API is stable, so
> we'd have to support this forever.  And you're putting C++ code into it,
> which is probably just wrong.
>
> If you can find an appropriate place in include/llvm/, maybe?

Duncan, perhaps include/llvm/IR/DataLayout.h is a good place for this?

--- a/include/llvm/IR/DataLayout.h
+++ b/include/llvm/IR/DataLayout.h
@@ -445,6 +445,16 @@ public:
   }
 };
 
+typedef struct LLVMOpaqueTargetData *LLVMTargetDataRef;
+
+inline DataLayout *unwrap(LLVMTargetDataRef P) {
+   return reinterpret_cast<DataLayout*>(P);
+}
+
+inline LLVMTargetDataRef wrap(const DataLayout *P) {
+   return reinterpret_cast<LLVMTargetDataRef>(const_cast<DataLayout*>(P));
+}
+
 class DataLayoutPass : public ImmutablePass {
   DataLayout DL;

--
That's together with the same changes in lib/Target/Target.cpp,
lib/Target/TargetMachineC.cpp,
lib/ExecutionEngine/ExecutionEngineBindings.cpp as in the original patch.