[PATCH] D45022: [X86] Mark all byval parameters as aliased

[Warren Ristow via Phabricator via llvm-commits]

wristow added inline comments.


================
Comment at: lib/Target/X86/X86ISelLowering.cpp:2836
     if (Bytes == 0) Bytes = 1; // Don't create zero-sized stack objects.
-    int FI = MFI.CreateFixedObject(Bytes, VA.getLocMemOffset(), isImmutable);
+    int FI = MFI.CreateFixedObject(Bytes, VA.getLocMemOffset(), isImmutable, true);
     // Adjust SP offset of interrupt parameter.
----------------
The code-change itself looks safe to me, and my understanding of the discussion in http://llvm.org/PR30290 is that this approach is conservative in terms of the aliasing that it will mark (but that identifying the precisely-minimal-aliasing will require a very large amount of restructuring).  My guess is that this conservative aliasing won't have a serious impact on overall code quality.  Like the FIXME comment above (line 2829), I think similar comment would be good.  Something like:

  // FIXME: For now, all byval parameter objects are marked as aliasing. This
  // can be improved with deeper analysis.

But I have to say I don't have a deep understanding of the area, so I'm curious to hear what others think.

Formatting nit: With the added `true` parameter, the line is more than 80 characters.
Also, I'd prefer a comment indicating what the parameter being set to `true` is doing:
    int FI = MFI.CreateFixedObject(Bytes, VA.getLocMemOffset(), isImmutable,
                                   /*isAliased=*/true);


https://reviews.llvm.org/D45022