[PATCH] D36556: [IR] AutoUpgrade ModuleFlagBehavior for PIC and PIE level
Steven Wu via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Thu Aug 17 13:13:24 PDT 2017
steven_wu added a comment.
In https://reviews.llvm.org/D36556#844596, @dexonsmith wrote:
> Who changed the behaviour? Can we get them to review the behaviour of the upgrade?
>
> I don't see a general test for PIE level. Can we add one of those as well?
The PIE test is in the same module.
test/Linker/module-flags-pic-1-a.ll needs to updated because the auto upgrade only happens when you load from bitcode. The texture format is not auto upgradeable and the old value still has valid syntax.
================
Comment at: lib/IR/AutoUpgrade.cpp:2273-2274
+ Type *Int32Ty = Type::getInt32Ty(Context);
+ MD->replaceOperandWith(
+ 0, ConstantAsMetadata::get(ConstantInt::get(Int32Ty, After)));
+ return true;
----------------
dexonsmith wrote:
> It's not generally sound to call `MDNode::replaceOperandWith`, unless the nodes are "distinct". I strongly suspect these nodes are "uniqued", which means that they are semantically constants (with far-too-lenient an API).
>
> Instead, you should construct a new node to attach to the `NamedMDNode`.
I am pretty sure the MDNode added to ModuleFlags are all uniqued.
I am trying to modify the ModuleFlag here. Attaching new node to module flag is easy but removing is not. Sure, I can reconstruct the entire module flag NamedMDNode, but I am not sure if that is worthwhile if the MDNode is ensured to be uniqued.
https://reviews.llvm.org/D36556
More information about the llvm-commits
mailing list