[PATCH] D98145: [FastISel] Don't trivially kill extractvalues (PR49467)

[Nikita Popov via Phabricator via llvm-commits]

nikic added inline comments.


================
Comment at: llvm/lib/CodeGen/SelectionDAG/FastISel.cpp:241
 
 bool FastISel::hasTrivialKill(const Value *V) {
   // Don't consider constants or arguments to have trivial kills.
----------------
spatel wrote:
> The header comment for this function only says:
>   /// Test whether the given value has exactly one use.
> 
> That should be updated with a little more detail about the logic and usage.
I expanded the comment a bit -- hopefully it makes sense, as I'm also not familiar with this code.


================
Comment at: llvm/test/CodeGen/X86/pr49467.ll:2
+; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py
+; RUN: llc -O0 -verify-machineinstrs -mtriple=x86_64 < %s | FileCheck %s
+
----------------
spatel wrote:
> Does it make sense to specify the point-of-failure in the RUN line more explicitly with something like:
>   -fast-isel -optimize-regalloc=0
I've added the `-fast-isel` flag, but don't think that `-optimize-regalloc=0` is really relevant here, in that the problem manifests before that and is caught by `-verify-machineinstrs` (although FastRA would also assert).


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

https://reviews.llvm.org/D98145