[PATCH] [x86] fix allowsMisalignedMemoryAccess() implementation

[Sanjay Patel]

================
Comment at: lib/Target/X86/X86ISelLowering.cpp:1802
@@ +1801,3 @@
+  if (Fast) {
+    if (Alignment == 0 || Alignment >= VT.getSizeInBits() / 8)
+      *Fast = true;
----------------
qcolombet wrote:
> jyknight wrote:
> > qcolombet wrote:
> > > Shouldn't we check the alignment from the data layout?
> > Why does this even have a check for correct alignment at all? The function is "allowsMisalignedMemoryAccesses" -- the assumption being you already know your data isn't aligned.
> > 
> > I think it's the caller's responsibility to call DataLayout::getPrefTypeAlignment, isn't it?
> > 
> > Perhaps the static "allowableAlignment" helper function in DAGCombiner.cpp should be made more generally available, to make doing so easier.
> My bad, you're right.
An audit of the trunk overrides of this function shows that only the SI lowering in the AMDGPU backend actually makes use of the Align param. Based on that implementation, it looks like the intended usage of the param is to specify how bad the misalignment can be (an 8-byte access with only 4-byte alignment is ok on that target).

There are a few spots in LegalizeDAG and SelectionDAG that check the DataLayout before calling allowsMisalignedMemoryAccesses(), so making allowableAlignment() more accessible sounds like a good change to me. I'll work on that and then fix this patch up.

Thanks!

http://reviews.llvm.org/D10662

EMAIL PREFERENCES
  http://reviews.llvm.org/settings/panel/emailpreferences/