[PATCH] Address Space Casting
Michele Scandale
michele.scandale at gmail.com
Mon Aug 19 13:33:34 PDT 2013
================
Comment at: lib/IR/Instructions.cpp:2299
@@ +2298,3 @@
+ "Illegal addrspacecast, bitcast sequence!");
+ // allowed, use first cast's opcode
+ return firstOp;
----------------
Matt Arsenault wrote:
> Comments should use proper capitalization and punctuation
Is the same as the comments in other cases (see case 1 and case 2)... Should I fix all the cases?
================
Comment at: lib/IR/Instructions.cpp:2308-2309
@@ +2307,4 @@
+ DstTy->isPointerTy() &&
+ SrcTy->getPointerAddressSpace() == MidTy->getPointerAddressSpace() &&
+ MidTy->getPointerAddressSpace() != DstTy->getPointerAddressSpace() &&
+ "Illegal bitcast, addrspacecast sequence!");
----------------
Matt Arsenault wrote:
> These look weirdly intended relative to the parts before it
Was a not so good choice to not break the 80 columns limit.
Is it better something like this?
assert(
SrcTy->isPointerTy() &&
MidTy->isPointerTy() &&
DstTy->isPointerTy() &&
SrcTy->getPointerAddressSpace() == MidTy->getPointerAddressSpace() &&
MidTy->getPointerAddressSpace() != DstTy->getPointerAddressSpace() &&
"Illegal bitcast, addrspacecast sequence!");
================
Comment at: lib/Target/X86/X86ISelLowering.h:766
@@ -765,1 +765,3 @@
+ virtual bool isNoopAddrSpaceCast(unsigned SrcAS, unsigned DestAS) const;
+
----------------
Matt Arsenault wrote:
> LLVM_OVERRIDE
This is not used on other similar cases... I just kept the local style.
================
Comment at: include/llvm/IR/Instructions.h:3755
@@ +3754,3 @@
+ Value *S, ///< The value to be casted
+ Type *Ty, ///< The type to casted to
+ const Twine &NameStr = "", ///< A name for the new instruction
----------------
Matt Arsenault wrote:
> This one isn't aligned with the rest, but the others are
Also the other CastInsts have this misalignement. I think that a separate patch should fix these code-style issues.
http://llvm-reviews.chandlerc.com/D1401
More information about the llvm-commits
mailing list