[PATCH] D88423: Fix llvm-link assert failure in BitCodeWriter

Mon Sep 28 11:54:45 PDT 2020

sanwou01 added a comment.

In D88423#2298681 <https://reviews.llvm.org/D88423#2298681>, @efriedma wrote:

> I'm not sure I understand the reason behind moving the EnumerateType call from incorporateFunction to the ValueEnumerator constructor.  We don't walk the attributes before that, do we?

In ModuleBitCodeWrite::write(), we call writeAttributeGroupTable() (which looks up the type of the attribute in VE's TypeMap) before writeFunction() (which calls VE::incorporateFunction()).

In D88423#2298842 <https://reviews.llvm.org/D88423#2298842>, @dblaikie wrote:

> Given the code has explicit cases for byval and sret separately - could you include test coverage for both as well? (maybe rename teh test case and both could be tested in the same file - on different parameters or different functions, etc)

Sure, I'll test both, but I think Eli's suggestion can get rid of the explicit byval and sret cases.

================
Comment at: llvm/lib/Bitcode/Writer/ValueEnumerator.cpp:408
+      if (A.hasAttribute(Attribute::ByVal))
+        EnumerateType(A.getParamByValType());
+
----------------
efriedma wrote:
> getPointeeInMemoryValueType()?
I figured there must be one that works for both, just had no idea which one. Thanks!

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D88423