[PATCH] D71213: [Alignment][NFC] CreateMemSet use MaybeAlign

2019-12-13 Thread Guillaume Chatelet via Phabricator via cfe-commits
gchatelet added a comment.

In D71213#1783467 , @nhaehnle wrote:

> In D71213#1781322 , @gchatelet wrote:
>
> > LLVM has this `LLVM_ATTRIBUTE_DEPRECATED` macro, it's convenient to get a 
> > warning but it only works when building without `-Wall`.
>
>
> Did you mean to write _with_ -Wall? I fail to see anything that stops the 
> macro from working with -Wall?
>
> Generally, I'd say using that macro a bit more for this kind of thing would 
> be a good idea.


Sorry I meant `-Werror` which will turn warnings into errors, then deprecation 
messages will break compilation.
I thought it was ON by default but it's not the case 
.
I'll go this route then. Thx!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71213



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D71213: [Alignment][NFC] CreateMemSet use MaybeAlign

2019-12-13 Thread Nicolai Hähnle via Phabricator via cfe-commits
nhaehnle added a comment.

In D71213#1781322 , @gchatelet wrote:

> LLVM has this `LLVM_ATTRIBUTE_DEPRECATED` macro, it's convenient to get a 
> warning but it only works when building without `-Wall`.


Did you mean to write _with_ -Wall? I fail to see anything that stops the macro 
from working with -Wall?

Generally, I'd say using that macro a bit more for this kind of thing would be 
a good idea.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71213



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D71213: [Alignment][NFC] CreateMemSet use MaybeAlign

2019-12-12 Thread Guillaume Chatelet via Phabricator via cfe-commits
gchatelet added a comment.

@foad do you have any insights on how to go with the deprecation?
LLVM has this `LLVM_ATTRIBUTE_DEPRECATED` macro, it's convenient to get a 
warning but it only works when building without `-Wall`.

Bots and in-tree users have it set by default, it's fine as I'll be fixing 
those but how about out of tree users?
Another option would be to have a comment but then nobody will actually update 
the code before it breaks.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71213



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D71213: [Alignment][NFC] CreateMemSet use MaybeAlign

2019-12-11 Thread Guillaume Chatelet via Phabricator via cfe-commits
gchatelet added a comment.

>> Thx for letting me know @foad . I'll make sure to keep the old API with a 
>> deprecation message from now on.
>>  Do you have any other suggestions on how to make this less painful for 
>> out-of-tree users? I'm afraid that the cleanup phase (removal of deprecated 
>> function) will be disruptive as well.
> 
> Removing deprecated functions is generally OK, as long as it happens *after* 
> the preferred function is introduced, so we have time to switch over.

Sure. My apologies for this.

> In the specific case of functions taking `Align` instead of `unsigned`, 
> perhaps you could start off allowing implicit conversion from `unsigned` to 
> `Align` and then remove it later, when all callers have been updated? Or 
> perhaps it's too late for that now.

I tried but it's not convenient for me to find the call sites that need 
updating. Also I don't want to introduce regressions by having automatic 
conversion from `bool` or anything implicitly convertible to `int`. By 
proceeding like this, I'm making sure that each change is reviewed and that the 
semantic is correct. When in doubt I try to get a review from the original 
author.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71213



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D71213: [Alignment][NFC] CreateMemSet use MaybeAlign

2019-12-11 Thread Jay Foad via Phabricator via cfe-commits
foad added a comment.

In D71213#1780088 , @gchatelet wrote:

> In D71213#1779841 , @foad wrote:
>
> > @gchatelet in general would it be possible to make changes like this in a 
> > backwards-compatible way, or in two stages without a "flag day" change? We 
> > have out-of-tree users of CreateMemSet and it's awkward to change them all 
> > at exactly the same time as we merge in this change from upstream llvm, and 
> > we have had the same problem with other MaybeAlign changes recently. I 
> > realise that LLVM doesn't make any official promises about API stability.
>
>
> Thx for letting me know @foad . I'll make sure to keep the old API with a 
> deprecation message from now on.
>  Do you have any other suggestions on how to make this less painful for 
> out-of-tree users? I'm afraid that the cleanup phase (removal of deprecated 
> function) will be disruptive as well.


Removing deprecated functions is generally OK, as long as it happens *after* 
the preferred function is introduced, so we have time to switch over.

In the specific case of functions taking `Align` instead of `unsigned`, perhaps 
you could start off allowing implicit conversion from `unsigned` to `Align` and 
then remove it later, when all callers have been updated? Or perhaps it's too 
late for that now.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71213



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D71213: [Alignment][NFC] CreateMemSet use MaybeAlign

2019-12-11 Thread Guillaume Chatelet via Phabricator via cfe-commits
gchatelet added a comment.

In D71213#1779841 , @foad wrote:

> @gchatelet in general would it be possible to make changes like this in a 
> backwards-compatible way, or in two stages without a "flag day" change? We 
> have out-of-tree users of CreateMemSet and it's awkward to change them all at 
> exactly the same time as we merge in this change from upstream llvm, and we 
> have had the same problem with other MaybeAlign changes recently. I realise 
> that LLVM doesn't make any official promises about API stability.


Thx for letting me know @foad . I'll make sure to keep the old API with a 
deprecation message from now on.
Do you have any other suggestions on how to make this less painful for 
out-of-tree users? I'm afraid that the cleanup phase (removal of deprecated 
function) will be disruptive as well.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71213



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D71213: [Alignment][NFC] CreateMemSet use MaybeAlign

2019-12-11 Thread Jay Foad via Phabricator via cfe-commits
foad added a comment.

@gchatelet in general would it be possible to make changes like this in a 
backwards-compatible way, or in two stages without a "flag day" change? We have 
out-of-tree users of CreateMemSet and it's awkward to change them all at 
exactly the same time as we merge in this change from upstream llvm, and we 
have had the same problem with other MaybeAlign changes recently. I realise 
that LLVM doesn't make any official promises about API stability.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71213



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D71213: [Alignment][NFC] CreateMemSet use MaybeAlign

2019-12-10 Thread Guillaume Chatelet via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rG1b2842bf902a: [Alignment][NFC] CreateMemSet use MaybeAlign 
(authored by gchatelet).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71213

Files:
  clang/lib/CodeGen/CGAtomic.cpp
  clang/lib/CodeGen/CGBuilder.h
  clang/lib/CodeGen/CGBuiltin.cpp
  llvm/include/llvm/IR/IRBuilder.h
  llvm/lib/IR/Core.cpp
  llvm/lib/IR/IRBuilder.cpp
  llvm/lib/Target/AMDGPU/AMDGPUPromoteAlloca.cpp
  llvm/lib/Transforms/Instrumentation/HWAddressSanitizer.cpp
  llvm/lib/Transforms/Instrumentation/MemorySanitizer.cpp
  llvm/lib/Transforms/Scalar/InferAddressSpaces.cpp
  llvm/lib/Transforms/Scalar/LoopIdiomRecognize.cpp
  llvm/lib/Transforms/Scalar/MemCpyOptimizer.cpp
  llvm/lib/Transforms/Scalar/SROA.cpp
  llvm/lib/Transforms/Utils/SimplifyLibCalls.cpp

Index: llvm/lib/Transforms/Utils/SimplifyLibCalls.cpp
===
--- llvm/lib/Transforms/Utils/SimplifyLibCalls.cpp
+++ llvm/lib/Transforms/Utils/SimplifyLibCalls.cpp
@@ -624,7 +624,7 @@
 
   if (SrcLen == 0) {
 // strncpy(x, "", y) -> memset(align 1 x, '\0', y)
-CallInst *NewCI = B.CreateMemSet(Dst, B.getInt8('\0'), Size, 1);
+CallInst *NewCI = B.CreateMemSet(Dst, B.getInt8('\0'), Size, Align::None());
 AttrBuilder ArgAttrs(CI->getAttributes().getParamAttributes(0));
 NewCI->setAttributes(NewCI->getAttributes().addParamAttributes(
 CI->getContext(), 0, ArgAttrs));
@@ -1235,7 +1235,8 @@
 
   // memset(p, v, n) -> llvm.memset(align 1 p, v, n)
   Value *Val = B.CreateIntCast(CI->getArgOperand(1), B.getInt8Ty(), false);
-  CallInst *NewCI = B.CreateMemSet(CI->getArgOperand(0), Val, Size, 1);
+  CallInst *NewCI =
+  B.CreateMemSet(CI->getArgOperand(0), Val, Size, Align::None());
   NewCI->setAttributes(CI->getAttributes());
   return CI->getArgOperand(0);
 }
@@ -3290,8 +3291,8 @@
 
   if (isFortifiedCallFoldable(CI, 3, 2)) {
 Value *Val = B.CreateIntCast(CI->getArgOperand(1), B.getInt8Ty(), false);
-CallInst *NewCI =
-B.CreateMemSet(CI->getArgOperand(0), Val, CI->getArgOperand(2), 1);
+CallInst *NewCI = B.CreateMemSet(CI->getArgOperand(0), Val,
+ CI->getArgOperand(2), Align::None());
 NewCI->setAttributes(CI->getAttributes());
 return CI->getArgOperand(0);
   }
Index: llvm/lib/Transforms/Scalar/SROA.cpp
===
--- llvm/lib/Transforms/Scalar/SROA.cpp
+++ llvm/lib/Transforms/Scalar/SROA.cpp
@@ -2801,7 +2801,7 @@
   Constant *Size = ConstantInt::get(SizeTy, NewEndOffset - NewBeginOffset);
   CallInst *New = IRB.CreateMemSet(
   getNewAllocaSlicePtr(IRB, OldPtr->getType()), II.getValue(), Size,
-  getSliceAlign(), II.isVolatile());
+  MaybeAlign(getSliceAlign()), II.isVolatile());
   if (AATags)
 New->setAAMetadata(AATags);
   LLVM_DEBUG(dbgs() << "  to: " << *New << "\n");
Index: llvm/lib/Transforms/Scalar/MemCpyOptimizer.cpp
===
--- llvm/lib/Transforms/Scalar/MemCpyOptimizer.cpp
+++ llvm/lib/Transforms/Scalar/MemCpyOptimizer.cpp
@@ -388,16 +388,12 @@
 StartPtr = Range.StartPtr;
 
 // Determine alignment
-unsigned Alignment = Range.Alignment;
-if (Alignment == 0) {
-  Type *EltType =
-cast(StartPtr->getType())->getElementType();
-  Alignment = DL.getABITypeAlignment(EltType);
-}
-
-AMemSet =
-  Builder.CreateMemSet(StartPtr, ByteVal, Range.End-Range.Start, Alignment);
+const Align Alignment = DL.getValueOrABITypeAlignment(
+MaybeAlign(Range.Alignment),
+cast(StartPtr->getType())->getElementType());
 
+AMemSet = Builder.CreateMemSet(StartPtr, ByteVal, Range.End - Range.Start,
+   Alignment);
 LLVM_DEBUG(dbgs() << "Replace stores:\n"; for (Instruction *SI
: Range.TheStores) dbgs()
   << *SI << '\n';
@@ -683,12 +679,11 @@
 auto *T = V->getType();
 if (T->isAggregateType()) {
   uint64_t Size = DL.getTypeStoreSize(T);
-  unsigned Align = SI->getAlignment();
-  if (!Align)
-Align = DL.getABITypeAlignment(T);
+  const Align MA =
+  DL.getValueOrABITypeAlignment(MaybeAlign(SI->getAlignment()), T);
   IRBuilder<> Builder(SI);
   auto *M =
-  Builder.CreateMemSet(SI->getPointerOperand(), ByteVal, Size, Align);
+  Builder.CreateMemSet(SI->getPointerOperand(), ByteVal, Size, MA);
 
   LLVM_DEBUG(dbgs() << "Promoting " << *SI << " to " << *M << "\n");
 
@@ -1058,7 +1053,7 @@
   Builder.CreateMemSet(
   Builder.CreateGEP(Dest->getType()->getPointerElementType(), Dest,
 SrcSize),
-  

[PATCH] D71213: [Alignment][NFC] CreateMemSet use MaybeAlign

2019-12-10 Thread Guillaume Chatelet via Phabricator via cfe-commits
gchatelet added a comment.

Thx for the review.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71213



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D71213: [Alignment][NFC] CreateMemSet use MaybeAlign

2019-12-10 Thread Guillaume Chatelet via Phabricator via cfe-commits
gchatelet updated this revision to Diff 233074.
gchatelet added a comment.

- Address comments


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71213

Files:
  clang/lib/CodeGen/CGAtomic.cpp
  clang/lib/CodeGen/CGBuilder.h
  clang/lib/CodeGen/CGBuiltin.cpp
  llvm/include/llvm/IR/IRBuilder.h
  llvm/lib/IR/Core.cpp
  llvm/lib/IR/IRBuilder.cpp
  llvm/lib/Target/AMDGPU/AMDGPUPromoteAlloca.cpp
  llvm/lib/Transforms/Instrumentation/HWAddressSanitizer.cpp
  llvm/lib/Transforms/Instrumentation/MemorySanitizer.cpp
  llvm/lib/Transforms/Scalar/InferAddressSpaces.cpp
  llvm/lib/Transforms/Scalar/LoopIdiomRecognize.cpp
  llvm/lib/Transforms/Scalar/MemCpyOptimizer.cpp
  llvm/lib/Transforms/Scalar/SROA.cpp
  llvm/lib/Transforms/Utils/SimplifyLibCalls.cpp

Index: llvm/lib/Transforms/Utils/SimplifyLibCalls.cpp
===
--- llvm/lib/Transforms/Utils/SimplifyLibCalls.cpp
+++ llvm/lib/Transforms/Utils/SimplifyLibCalls.cpp
@@ -624,7 +624,7 @@
 
   if (SrcLen == 0) {
 // strncpy(x, "", y) -> memset(align 1 x, '\0', y)
-CallInst *NewCI = B.CreateMemSet(Dst, B.getInt8('\0'), Size, 1);
+CallInst *NewCI = B.CreateMemSet(Dst, B.getInt8('\0'), Size, Align::None());
 AttrBuilder ArgAttrs(CI->getAttributes().getParamAttributes(0));
 NewCI->setAttributes(NewCI->getAttributes().addParamAttributes(
 CI->getContext(), 0, ArgAttrs));
@@ -1235,7 +1235,8 @@
 
   // memset(p, v, n) -> llvm.memset(align 1 p, v, n)
   Value *Val = B.CreateIntCast(CI->getArgOperand(1), B.getInt8Ty(), false);
-  CallInst *NewCI = B.CreateMemSet(CI->getArgOperand(0), Val, Size, 1);
+  CallInst *NewCI =
+  B.CreateMemSet(CI->getArgOperand(0), Val, Size, Align::None());
   NewCI->setAttributes(CI->getAttributes());
   return CI->getArgOperand(0);
 }
@@ -3290,8 +3291,8 @@
 
   if (isFortifiedCallFoldable(CI, 3, 2)) {
 Value *Val = B.CreateIntCast(CI->getArgOperand(1), B.getInt8Ty(), false);
-CallInst *NewCI =
-B.CreateMemSet(CI->getArgOperand(0), Val, CI->getArgOperand(2), 1);
+CallInst *NewCI = B.CreateMemSet(CI->getArgOperand(0), Val,
+ CI->getArgOperand(2), Align::None());
 NewCI->setAttributes(CI->getAttributes());
 return CI->getArgOperand(0);
   }
Index: llvm/lib/Transforms/Scalar/SROA.cpp
===
--- llvm/lib/Transforms/Scalar/SROA.cpp
+++ llvm/lib/Transforms/Scalar/SROA.cpp
@@ -2801,7 +2801,7 @@
   Constant *Size = ConstantInt::get(SizeTy, NewEndOffset - NewBeginOffset);
   CallInst *New = IRB.CreateMemSet(
   getNewAllocaSlicePtr(IRB, OldPtr->getType()), II.getValue(), Size,
-  getSliceAlign(), II.isVolatile());
+  MaybeAlign(getSliceAlign()), II.isVolatile());
   if (AATags)
 New->setAAMetadata(AATags);
   LLVM_DEBUG(dbgs() << "  to: " << *New << "\n");
Index: llvm/lib/Transforms/Scalar/MemCpyOptimizer.cpp
===
--- llvm/lib/Transforms/Scalar/MemCpyOptimizer.cpp
+++ llvm/lib/Transforms/Scalar/MemCpyOptimizer.cpp
@@ -388,16 +388,12 @@
 StartPtr = Range.StartPtr;
 
 // Determine alignment
-unsigned Alignment = Range.Alignment;
-if (Alignment == 0) {
-  Type *EltType =
-cast(StartPtr->getType())->getElementType();
-  Alignment = DL.getABITypeAlignment(EltType);
-}
-
-AMemSet =
-  Builder.CreateMemSet(StartPtr, ByteVal, Range.End-Range.Start, Alignment);
+const Align Alignment = DL.getValueOrABITypeAlignment(
+MaybeAlign(Range.Alignment),
+cast(StartPtr->getType())->getElementType());
 
+AMemSet = Builder.CreateMemSet(StartPtr, ByteVal, Range.End - Range.Start,
+   Alignment);
 LLVM_DEBUG(dbgs() << "Replace stores:\n"; for (Instruction *SI
: Range.TheStores) dbgs()
   << *SI << '\n';
@@ -683,12 +679,11 @@
 auto *T = V->getType();
 if (T->isAggregateType()) {
   uint64_t Size = DL.getTypeStoreSize(T);
-  unsigned Align = SI->getAlignment();
-  if (!Align)
-Align = DL.getABITypeAlignment(T);
+  const Align MA =
+  DL.getValueOrABITypeAlignment(MaybeAlign(SI->getAlignment()), T);
   IRBuilder<> Builder(SI);
   auto *M =
-  Builder.CreateMemSet(SI->getPointerOperand(), ByteVal, Size, Align);
+  Builder.CreateMemSet(SI->getPointerOperand(), ByteVal, Size, MA);
 
   LLVM_DEBUG(dbgs() << "Promoting " << *SI << " to " << *M << "\n");
 
@@ -1058,7 +1053,7 @@
   Builder.CreateMemSet(
   Builder.CreateGEP(Dest->getType()->getPointerElementType(), Dest,
 SrcSize),
-  MemSet->getOperand(1), MemsetLen, Align);
+  MemSet->getOperand(1), MemsetLen, MaybeAlign(Align));
 

[PATCH] D71213: [Alignment][NFC] CreateMemSet use MaybeAlign

2019-12-10 Thread Guillaume Chatelet via Phabricator via cfe-commits
gchatelet marked 2 inline comments as done.
gchatelet added inline comments.



Comment at: llvm/lib/Transforms/Instrumentation/MemorySanitizer.cpp:2910
+const Align Alignment =
+assumeAligned(cast(I.getArgOperand(2))->getZExtValue());
 Value *Mask = I.getArgOperand(3);

courbet wrote:
> how did you infer this ? I'm not sure I can prove that this is nonzero.
The argument can be `0` and in that case `assumeAligned` will turn it into `1`.
It's not a problem for `getShadowOriginPtr` which treats `0` as `1`.
It does change the generated IR for `CreateMaskedStore` which will have an 
alignment of `1` instead of `0`. But I believe this is treated the same way 
down the road by the CodeGen.
All tests are still passing.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71213



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D71213: [Alignment][NFC] CreateMemSet use MaybeAlign

2019-12-10 Thread Clement Courbet via Phabricator via cfe-commits
courbet added inline comments.



Comment at: llvm/lib/Transforms/Instrumentation/MemorySanitizer.cpp:1092
 const DataLayout  = F.getParent()->getDataLayout();
-unsigned IntptrAlignment = DL.getABITypeAlignment(MS.IntptrTy);
+Align IntptrAlignment = Align(DL.getABITypeAlignment(MS.IntptrTy));
 unsigned IntptrSize = DL.getTypeStoreSize(MS.IntptrTy);

const ?



Comment at: llvm/lib/Transforms/Instrumentation/MemorySanitizer.cpp:2910
+const Align Alignment =
+assumeAligned(cast(I.getArgOperand(2))->getZExtValue());
 Value *Mask = I.getArgOperand(3);

how did you infer this ? I'm not sure I can prove that this is nonzero.



Comment at: llvm/lib/Transforms/Instrumentation/MemorySanitizer.cpp:2940
+const Align Alignment =
+assumeAligned(cast(I.getArgOperand(1))->getZExtValue());
 Value *Mask = I.getArgOperand(2);

ditto



Comment at: llvm/lib/Transforms/Instrumentation/MemorySanitizer.cpp:3319
 if (ArgOffset + Size > kParamTLSSize) break;
-unsigned ParamAlignment = CS.getParamAlignment(i);
-unsigned Alignment = std::min(ParamAlignment, kShadowTLSAlignment);
+const Align ParamAlignment = assumeAligned(CS.getParamAlignment(i));
+const Align Alignment = std::min(ParamAlignment, kShadowTLSAlignment);

ditto


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71213



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D71213: [Alignment][NFC] CreateMemSet use MaybeAlign

2019-12-09 Thread Guillaume Chatelet via Phabricator via cfe-commits
gchatelet created this revision.
gchatelet added a reviewer: courbet.
Herald added subscribers: llvm-commits, cfe-commits, hiraditya, nhaehnle, 
jvesely, arsenm.
Herald added projects: clang, LLVM.

This is patch is part of a series to introduce an Alignment type.
See this thread for context: 
http://lists.llvm.org/pipermail/llvm-dev/2019-July/133851.html
See this patch for the introduction of the type: https://reviews.llvm.org/D64790


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D71213

Files:
  clang/lib/CodeGen/CGAtomic.cpp
  clang/lib/CodeGen/CGBuilder.h
  clang/lib/CodeGen/CGBuiltin.cpp
  llvm/include/llvm/IR/IRBuilder.h
  llvm/lib/IR/Core.cpp
  llvm/lib/IR/IRBuilder.cpp
  llvm/lib/Target/AMDGPU/AMDGPUPromoteAlloca.cpp
  llvm/lib/Transforms/Instrumentation/HWAddressSanitizer.cpp
  llvm/lib/Transforms/Instrumentation/MemorySanitizer.cpp
  llvm/lib/Transforms/Scalar/InferAddressSpaces.cpp
  llvm/lib/Transforms/Scalar/LoopIdiomRecognize.cpp
  llvm/lib/Transforms/Scalar/MemCpyOptimizer.cpp
  llvm/lib/Transforms/Scalar/SROA.cpp
  llvm/lib/Transforms/Utils/SimplifyLibCalls.cpp

Index: llvm/lib/Transforms/Utils/SimplifyLibCalls.cpp
===
--- llvm/lib/Transforms/Utils/SimplifyLibCalls.cpp
+++ llvm/lib/Transforms/Utils/SimplifyLibCalls.cpp
@@ -624,7 +624,7 @@
 
   if (SrcLen == 0) {
 // strncpy(x, "", y) -> memset(align 1 x, '\0', y)
-CallInst *NewCI = B.CreateMemSet(Dst, B.getInt8('\0'), Size, 1);
+CallInst *NewCI = B.CreateMemSet(Dst, B.getInt8('\0'), Size, Align::None());
 AttrBuilder ArgAttrs(CI->getAttributes().getParamAttributes(0));
 NewCI->setAttributes(NewCI->getAttributes().addParamAttributes(
 CI->getContext(), 0, ArgAttrs));
@@ -1235,7 +1235,8 @@
 
   // memset(p, v, n) -> llvm.memset(align 1 p, v, n)
   Value *Val = B.CreateIntCast(CI->getArgOperand(1), B.getInt8Ty(), false);
-  CallInst *NewCI = B.CreateMemSet(CI->getArgOperand(0), Val, Size, 1);
+  CallInst *NewCI =
+  B.CreateMemSet(CI->getArgOperand(0), Val, Size, Align::None());
   NewCI->setAttributes(CI->getAttributes());
   return CI->getArgOperand(0);
 }
@@ -3290,8 +3291,8 @@
 
   if (isFortifiedCallFoldable(CI, 3, 2)) {
 Value *Val = B.CreateIntCast(CI->getArgOperand(1), B.getInt8Ty(), false);
-CallInst *NewCI =
-B.CreateMemSet(CI->getArgOperand(0), Val, CI->getArgOperand(2), 1);
+CallInst *NewCI = B.CreateMemSet(CI->getArgOperand(0), Val,
+ CI->getArgOperand(2), Align::None());
 NewCI->setAttributes(CI->getAttributes());
 return CI->getArgOperand(0);
   }
Index: llvm/lib/Transforms/Scalar/SROA.cpp
===
--- llvm/lib/Transforms/Scalar/SROA.cpp
+++ llvm/lib/Transforms/Scalar/SROA.cpp
@@ -2801,7 +2801,7 @@
   Constant *Size = ConstantInt::get(SizeTy, NewEndOffset - NewBeginOffset);
   CallInst *New = IRB.CreateMemSet(
   getNewAllocaSlicePtr(IRB, OldPtr->getType()), II.getValue(), Size,
-  getSliceAlign(), II.isVolatile());
+  MaybeAlign(getSliceAlign()), II.isVolatile());
   if (AATags)
 New->setAAMetadata(AATags);
   LLVM_DEBUG(dbgs() << "  to: " << *New << "\n");
Index: llvm/lib/Transforms/Scalar/MemCpyOptimizer.cpp
===
--- llvm/lib/Transforms/Scalar/MemCpyOptimizer.cpp
+++ llvm/lib/Transforms/Scalar/MemCpyOptimizer.cpp
@@ -388,16 +388,12 @@
 StartPtr = Range.StartPtr;
 
 // Determine alignment
-unsigned Alignment = Range.Alignment;
-if (Alignment == 0) {
-  Type *EltType =
-cast(StartPtr->getType())->getElementType();
-  Alignment = DL.getABITypeAlignment(EltType);
-}
-
-AMemSet =
-  Builder.CreateMemSet(StartPtr, ByteVal, Range.End-Range.Start, Alignment);
+const Align Alignment = DL.getValueOrABITypeAlignment(
+MaybeAlign(Range.Alignment),
+cast(StartPtr->getType())->getElementType());
 
+AMemSet = Builder.CreateMemSet(StartPtr, ByteVal, Range.End - Range.Start,
+   Alignment);
 LLVM_DEBUG(dbgs() << "Replace stores:\n"; for (Instruction *SI
: Range.TheStores) dbgs()
   << *SI << '\n';
@@ -683,12 +679,11 @@
 auto *T = V->getType();
 if (T->isAggregateType()) {
   uint64_t Size = DL.getTypeStoreSize(T);
-  unsigned Align = SI->getAlignment();
-  if (!Align)
-Align = DL.getABITypeAlignment(T);
+  const Align MA =
+  DL.getValueOrABITypeAlignment(MaybeAlign(SI->getAlignment()), T);
   IRBuilder<> Builder(SI);
   auto *M =
-  Builder.CreateMemSet(SI->getPointerOperand(), ByteVal, Size, Align);
+  Builder.CreateMemSet(SI->getPointerOperand(), ByteVal, Size, MA);
 
   LLVM_DEBUG(dbgs() << "Promoting " << *SI << " to " << *M