[PATCH] D135738: [InstCombine] Bail out of casting calls when a conversion to byval is involved.

[Nikita Popov via Phabricator via llvm-commits]

nikic added a comment.

Okay, I looked a bit closer into what clang is doing here with the intention of implementing the same in rustc. This turns out to be more complicated than I originally thought (some notes in https://github.com/rust-lang/rust/issues/102174#issuecomment-1285502667). I believe this would require an entirely new PassMode, which can model the unpacked struct plus a possibly needed inreg padding register.

With that in mind, I'm okay with landing this mitigation in the meantime, especially as this does not lose any practical transformation power.



================
Comment at: llvm/lib/Transforms/InstCombine/InstCombineCalls.cpp:3400
+        Callee->getAttributes().hasParamAttr(i, Attribute::ByVal))
+      return false; // Cannot transform to byval.
+
----------------
Do we need to prevent the reverse direction as well? I.e. call-site has byval, function doesn't.


================
Comment at: llvm/test/Transforms/InstCombine/cast-to-byval.ll:2
+; Check that function calls involving conversion to byval aren't transformed.
+; RUN: opt < %s -passes=instcombine -S | FileCheck %s
+
----------------
Use update_test_checks.py.


================
Comment at: llvm/test/Transforms/InstCombine/cast-to-byval.ll:4-5
+
+target datalayout = "e-m:e-p:32:32-p270:32:32-p271:32:32-p272:64:64-f64:32:64-f80:32-n8:16:32-S128"
+target triple = "i686-unknown-linux-gnu"
+
----------------
glandium wrote:
> arsenm wrote:
> > Shouldn't need this
> The test passes with the unpatched code without this.
You need to change to an i64 argument when dropping the data layout, so it matches the ptr size.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D135738