[PATCH] D93708: [AMDGPU] Add a new Clamp Pattern to the GlobalISel Path.
Matt Arsenault via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Wed Dec 23 07:15:24 PST 2020
arsenm added inline comments.
================
Comment at: llvm/include/llvm/CodeGen/GlobalISel/MIPatternMatch.h:475
+template <typename Boundary1, typename Boundary2, typename Origin>
+struct MaxMin_match_helper {
+ Boundary1 B1;
----------------
This is doing too much, this should just match the G_UMIN/G_UMAX/G_SMIN/G_SMAX opcodes. This shouldn't be responsible for matching the compare+select pattern
================
Comment at: llvm/test/CodeGen/AMDGPU/GlobalISel/clampi64toi16.ll:1
+; RUN: llc -global-isel -mcpu=tahiti -mtriple=amdgcn-amd-amdhsa -verify-machineinstrs < %s | FileCheck --check-prefixes=GFX678,GFX6789 %s
+; RUN: llc -global-isel -mcpu=gfx900 -mtriple=amdgcn-amd-amdhsa -verify-machineinstrs < %s | FileCheck --check-prefixes=GFX9,GFX6789 %s
----------------
I would prefer if the test name had a more consistent name. How about combine-int-clamp.ll?
================
Comment at: llvm/test/CodeGen/AMDGPU/GlobalISel/clampi64toi16.ll:18-21
+ %2 = icmp slt i64 %1, 32767
+ %3 = select i1 %2, i64 %1, i64 32767
+ %4 = trunc i64 %3 to i16
+
----------------
I dislike anonymous values in tests, can you add names here
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D93708/new/
https://reviews.llvm.org/D93708
More information about the llvm-commits
mailing list