[PATCH] D81728: [InstCombine] Add target-specific inst combining

Flakebi added a comment. Thanks for the notification @davezarzycki, an auto-bisecting bot is cool! This failure should be fixed in b99898c1e9c5d8bade1d898e84604d3241b0087c . Repository: rG LLVM Github Monor

[PATCH] D81728: [InstCombine] Add target-specific inst combining

davezarzycki added a comment. I have a multi-stage, auto-git-bisecting bot that has identifying this commit as the source of a regression on Fedora 32 (x86-64). This commit broke my first stage test (release, no asserts). Might a quick fix happen or do we need to revert this? FAIL: Clang ::

[PATCH] D81728: [InstCombine] Add target-specific inst combining

nhaehnle accepted this revision. nhaehnle added a comment. This revision is now accepted and ready to land. This has had a month of good review that has been addressed, I'd say it's good to go. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D81728/ne

[PATCH] D81728: [InstCombine] Add target-specific inst combining

dmgreen added a comment. Thanks Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D81728/new/ https://reviews.llvm.org/D81728 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/c

[PATCH] D81728: [InstCombine] Add target-specific inst combining

dmgreen added inline comments. Comment at: llvm/test/CodeGen/Thumb2/mve-intrinsics/predicates.ll:2 ; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py -; RUN: opt -instcombine %s | llc -mtriple=thumbv8.1m.main -mattr=+mve.fp -verify-machineinstrs -o -

[PATCH] D81728: [InstCombine] Add target-specific inst combining

foad added inline comments. Comment at: llvm/include/llvm/Analysis/TargetTransformInfo.h:552-555 + /// \returns false to not do anything target specific or true to return the + /// value in \p ResultI from the InstCombiner. It is possible to return null + /// and stop further

[PATCH] D81728: [InstCombine] Add target-specific inst combining

Flakebi marked an inline comment as done. Flakebi added inline comments. Comment at: llvm/include/llvm/Analysis/TargetTransformInfo.h:542 + bool simplifyDemandedUseBitsIntrinsic(InstCombiner &IC, IntrinsicInst &II, +APInt DemandedMask, Kno

[PATCH] D81728: [InstCombine] Add target-specific inst combining

lattner resigned from this revision. lattner added a comment. Please don't consider me a blocker on this patch, thank you for pushing on it! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D81728/new/ https://reviews.llvm.org/D81728 ___

[PATCH] D81728: [InstCombine] Add target-specific inst combining

Flakebi marked an inline comment as done. Flakebi added inline comments. Comment at: llvm/include/llvm/Analysis/TargetTransformInfo.h:540 + bool instCombineIntrinsic(InstCombiner &IC, IntrinsicInst &II, +Instruction **ResultI) const; + bool simplifyD

[PATCH] D81728: [InstCombine] Add target-specific inst combining

nikic added inline comments. Comment at: llvm/include/llvm/Analysis/TargetTransformInfo.h:540 + bool instCombineIntrinsic(InstCombiner &IC, IntrinsicInst &II, +Instruction **ResultI) const; + bool simplifyDemandedUseBitsIntrinsic(InstCombiner &IC, In

[PATCH] D81728: [InstCombine] Add target-specific inst combining

nhaehnle added inline comments. Comment at: llvm/include/llvm/Transforms/InstCombine/InstCombiner.h:46 +/// combine them. +class LLVM_LIBRARY_VISIBILITY InstCombiner { +public: lattner wrote: > I would really rather not make this be a public class - this is a ver

[PATCH] D81728: [InstCombine] Add target-specific inst combining

lattner requested changes to this revision. lattner added a comment. This revision now requires changes to proceed. This looks like a great direction, but please make sure to minimize public implementation details. We don't want the vast majority of instcombine to be visible outside of its libr

[PATCH] D81728: [InstCombine] Add target-specific inst combining

foad added a subscriber: bogner. foad added inline comments. Comment at: llvm/lib/Transforms/InstCombine/InstCombineSimplifyDemanded.cpp:1444 + *this, *II, DemandedElts, UndefElts, UndefElts2, UndefElts3, + simplifyAndSetOp, &V)) +return V; -

[PATCH] D81728: [InstCombine] Add target-specific inst combining

efriedma added a comment. > This combines instructions, so I think it belongs into the InstCombine pass. > On the other hand, the f16 form of the intrinsics is not available on all > targets, so this combination cannot be applied unconditionally but it needs > to be gated depending on the targe

[PATCH] D81728: [InstCombine] Add target-specific inst combining

arsenm added a comment. I think an interface usable by InstructionSimplify would be helpful too, so I think that would be a separate thing from TTI Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D81728/new/ https://reviews.llvm.org/D81728 ___

[PATCH] D81728: [InstCombine] Add target-specific inst combining

nhaehnle added a comment. We've been handling target-specific intrinsics in InstCombine for a long time, and that's the place where they should naturally sit. This is a pretty clean refactoring in my opinion, I'm in favor. It's substantial enough as a change that it should probably receive a he