[PATCH] D116971: [AttributorAttributes] Remove hardcoded parameters

[Johannes Doerfert via Phabricator via llvm-commits]

jdoerfert added a comment.

> Is it possible to get away without adding getAllocSizeArgs to MemoryBuiltins. Based on some email conversations that I have had, getObjectSize would be prefered but in this case, we want to get the size arguments even if actual size is not a known constant. It's also used to get one of the types for the memset on line 5999. I don't fully understand what that type is doing so there may be a better way to do that.

I think h2s for non-constant size allocations should be possible (through a user switch). Thus, we should not require a constant object size. We might even use our own constant value info later to get an upper bound for non-constant allocations. 
The memset type things is an artifact though. The idea is that the type used for the size parameter must be good enough for the memset size parameter, though, we probably can use some other type as well.

> If the alignment is specified but is not a known constant, I use the maximum alignment as the alignment. Is this acceptable or should we use the minimum alignment or perhaps there is some way or change that could allow the alignment to be dynamic.

Setting the alignment to maximum doesn't make sense (and won't work). Minimum, or any other value that is too low, won't work either. So the alignment actually has to be a constant, or at least bounded by a (small) constant.



================
Comment at: llvm/lib/Transforms/IPO/AttributorAttributes.cpp:5967
+        } else {
+          Alignment = max(Alignment, MaybeAlign(llvm::Value::MaximumAlignment));
+        }
----------------
I'm not sure I understand why we take the maximum of something and the MaximumAlignment?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D116971