llvm / llvm-project

The LLVM Project is a collection of modular and reusable compiler and toolchain technologies.
http://llvm.org
Other
28.87k stars 11.93k forks source link

missed fold, sext(fptosi x to i32) => fptosi x to i64 #85268

Open zhengyang92 opened 7 months ago

zhengyang92 commented 7 months ago

https://alive2.llvm.org/ce/z/bFW9vZ online Alive timed out for this one, you'll need local alive2 with larger smt-to.

define i64 @src(float %0) {
if.end27:
%1 = fptosi float %0 to i32
%2 = sext i32 %1 to i64
ret i64 %2

sink: ; No predecessors!
unreachable
}

define i64 @tgt(float %0) {
if.end27:
%1 = fptosi float %0 to i64
ret i64 %1

sink: ; No predecessors!
unreachable
}

no brainer, same pattern applies to fptoui.

zhengyang92 commented 7 months ago

@jayfoad @regehr

llvmbot commented 7 months ago

Hi!

This issue may be a good introductory issue for people new to working on LLVM. If you would like to work on this issue, your first steps are:

  1. In the comments of the issue, request for it to be assigned to you.
  2. Fix the issue locally.
  3. Run the test suite locally. Remember that the subdirectories under test/ create fine-grained testing targets, so you can e.g. use make check-clang-ast to only run Clang's AST tests.
  4. Create a Git commit.
  5. Run git clang-format HEAD~1 to format your changes.
  6. Open a pull request to the upstream repository on GitHub. Detailed instructions can be found in GitHub's documentation.

If you have any further questions about this issue, don't hesitate to ask via a comment in the thread below.

llvmbot commented 7 months ago

@llvm/issue-subscribers-good-first-issue

Author: Zhengyang Liu (zhengyang92)

https://alive2.llvm.org/ce/z/bFW9vZ online Alive timed out for this one, you'll need local alive2 with larger smt-to. ```llvm define i64 @src(float %0) { if.end27: %1 = fptosi float %0 to i32 %2 = sext i32 %1 to i64 ret i64 %2 sink: ; No predecessors! unreachable } define i64 @tgt(float %0) { if.end27: %1 = fptosi float %0 to i64 ret i64 %1 sink: ; No predecessors! unreachable } ``` no brainer, same pattern applies to fptoui.
jayfoad commented 7 months ago

My feeling is that we probably should not do this in IR because:

Instead it could be done as a target-specific DAG combine or instruction selection pattern.

nikic commented 7 months ago

I agree that we should not do this in IR.

zhengyang92 commented 7 months ago

@nikic @jayfoad agreed.

miguelraz commented 7 months ago

I'd like to work on this issue if possible 😀

HendrikHuebner commented 6 months ago

@miguelraz Are you still planning to work on this? Otherwise I'd like pick it up.

miguelraz commented 6 months ago

@HendrikHuebner Yes I am, sorry for licking the cookie for a bit - I'm just drowning in exams at the moment.

wizardengineer commented 3 months ago

Is this assuming that sext will always be the next instruction?

arsenm commented 3 months ago

Is this assuming that sext will always be the next instruction?

The ordering / placement of instructions does not matter. The combine would be rooted at the sext and look at its operand

c8ef commented 2 months ago

@miguelraz are you still working on this?

c8ef commented 2 months ago

Ping. @miguelraz