[all-commits] [llvm/llvm-project] 5eb8cb: [NFC][GlobalISel] Don't return `bool` from apply f...

Pierre van Houtryve via All-commits all-commits at lists.llvm.org
Mon Jun 26 00:24:11 PDT 2023


  Branch: refs/heads/main
  Home:   https://github.com/llvm/llvm-project
  Commit: 5eb8cb094981f17364d5e7689b9e284918598a11
      https://github.com/llvm/llvm-project/commit/5eb8cb094981f17364d5e7689b9e284918598a11
  Author: pvanhout <pierre.vanhoutryve at amd.com>
  Date:   2023-06-26 (Mon, 26 Jun 2023)

  Changed paths:
    M llvm/include/llvm/CodeGen/GlobalISel/CombinerHelper.h
    M llvm/include/llvm/Target/GlobalISel/Combine.td
    M llvm/lib/CodeGen/GlobalISel/CombinerHelper.cpp
    M llvm/lib/Target/AArch64/AArch64Combine.td
    M llvm/lib/Target/AArch64/GISel/AArch64PostLegalizerCombiner.cpp
    M llvm/lib/Target/AArch64/GISel/AArch64PostLegalizerLowering.cpp
    M llvm/lib/Target/AArch64/GISel/AArch64PreLegalizerCombiner.cpp

  Log Message:
  -----------
  [NFC][GlobalISel] Don't return `bool` from apply functions

There is no case where those functions return false. It's always return true.
Even if they were to return false, it's not really something we should rely on I think.
With the current combiner implementation, it would just make `tryCombineAll` return false without retrying anymore rules.

I also believe that if an applyer were to return false, it would mean that the match function is not good enough. Asserting on failure in an apply function is a better idea, IMO.

Reviewed By: arsenm

Differential Revision: https://reviews.llvm.org/D153619




More information about the All-commits mailing list