[clang] [llvm] [InstCombine] Infer nsw/nuw for trunc (PR #87910)

[Yingwei Zheng via cfe-commits]

https://github.com/dtcxzyw closed 
https://github.com/llvm/llvm-project/pull/87910
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [llvm] [InstCombine] Infer nsw/nuw for trunc (PR #87910)

[Nikita Popov via cfe-commits]

@@ -897,7 +897,20 @@ Instruction *InstCombinerImpl::visitTrunc(TruncInst 
) {
 }
   }
 
-  return nullptr;
+  bool Changed = false;
+  if (!Trunc.hasNoSignedWrap() &&
+  ComputeMaxSignificantBits(Src, /*Depth=*/0, ) <= DestWidth) {
+Trunc.setHasNoSignedWrap(true);
+Changed = true;
+  }
+  if (!Trunc.hasNoUnsignedWrap() &&
+  MaskedValueIsZero(Src, APInt::getBitsSetFrom(SrcWidth, DestWidth),
+/*Depth=*/0, )) {
+Trunc.setHasNoUnsignedWrap(true);
+Changed = true;

nikic wrote:

Okay, I won't insist on this.

https://github.com/llvm/llvm-project/pull/87910
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [llvm] [InstCombine] Infer nsw/nuw for trunc (PR #87910)

[Nikita Popov via cfe-commits]

https://github.com/nikic approved this pull request.

LGTM

https://github.com/llvm/llvm-project/pull/87910
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [llvm] [InstCombine] Infer nsw/nuw for trunc (PR #87910)

[Nikita Popov via cfe-commits]

https://github.com/nikic edited https://github.com/llvm/llvm-project/pull/87910
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [llvm] [InstCombine] Infer nsw/nuw for trunc (PR #87910)

[Yingwei Zheng via cfe-commits]

@@ -897,7 +897,20 @@ Instruction *InstCombinerImpl::visitTrunc(TruncInst 
) {
 }
   }
 
-  return nullptr;
+  bool Changed = false;
+  if (!Trunc.hasNoSignedWrap() &&
+  ComputeMaxSignificantBits(Src, /*Depth=*/0, ) <= DestWidth) {
+Trunc.setHasNoSignedWrap(true);
+Changed = true;
+  }
+  if (!Trunc.hasNoUnsignedWrap() &&
+  MaskedValueIsZero(Src, APInt::getBitsSetFrom(SrcWidth, DestWidth),
+/*Depth=*/0, )) {
+Trunc.setHasNoUnsignedWrap(true);
+Changed = true;

dtcxzyw wrote:

Ping @nikic


https://github.com/llvm/llvm-project/pull/87910
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [llvm] [InstCombine] Infer nsw/nuw for trunc (PR #87910)

[Yingwei Zheng via cfe-commits]

@@ -897,7 +897,20 @@ Instruction *InstCombinerImpl::visitTrunc(TruncInst 
) {
 }
   }
 
-  return nullptr;
+  bool Changed = false;
+  if (!Trunc.hasNoSignedWrap() &&
+  ComputeMaxSignificantBits(Src, /*Depth=*/0, ) <= DestWidth) {
+Trunc.setHasNoSignedWrap(true);
+Changed = true;
+  }
+  if (!Trunc.hasNoUnsignedWrap() &&
+  MaskedValueIsZero(Src, APInt::getBitsSetFrom(SrcWidth, DestWidth),
+/*Depth=*/0, )) {
+Trunc.setHasNoUnsignedWrap(true);
+Changed = true;

dtcxzyw wrote:

> We can't infer nsw, but we can infer nuw.

I prefer to infer both flags here, then we may reuse KnownBits in further 
patches.

> Do you see any reason why doing this in SimplifyDemanded would be problematic?

`SimplifyDemanded` is context-sensitive. IMO it is not the right place to infer 
flags.


https://github.com/llvm/llvm-project/pull/87910
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [llvm] [InstCombine] Infer nsw/nuw for trunc (PR #87910)

[Nikita Popov via cfe-commits]

@@ -897,7 +897,20 @@ Instruction *InstCombinerImpl::visitTrunc(TruncInst 
) {
 }
   }
 
-  return nullptr;
+  bool Changed = false;
+  if (!Trunc.hasNoSignedWrap() &&
+  ComputeMaxSignificantBits(Src, /*Depth=*/0, ) <= DestWidth) {
+Trunc.setHasNoSignedWrap(true);
+Changed = true;
+  }
+  if (!Trunc.hasNoUnsignedWrap() &&
+  MaskedValueIsZero(Src, APInt::getBitsSetFrom(SrcWidth, DestWidth),
+/*Depth=*/0, )) {
+Trunc.setHasNoUnsignedWrap(true);
+Changed = true;

nikic wrote:

We can't infer nsw, but we can infer nuw. Do you see any reason why doing this 
in SimplifyDemanded would be problematic?

https://github.com/llvm/llvm-project/pull/87910
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [llvm] [InstCombine] Infer nsw/nuw for trunc (PR #87910)

[Yingwei Zheng via cfe-commits]

@@ -897,7 +897,20 @@ Instruction *InstCombinerImpl::visitTrunc(TruncInst 
) {
 }
   }
 
-  return nullptr;
+  bool Changed = false;
+  if (!Trunc.hasNoSignedWrap() &&
+  ComputeMaxSignificantBits(Src, /*Depth=*/0, ) <= DestWidth) {
+Trunc.setHasNoSignedWrap(true);
+Changed = true;
+  }
+  if (!Trunc.hasNoUnsignedWrap() &&
+  MaskedValueIsZero(Src, APInt::getBitsSetFrom(SrcWidth, DestWidth),
+/*Depth=*/0, )) {
+Trunc.setHasNoUnsignedWrap(true);
+Changed = true;

dtcxzyw wrote:

We cannot infer nsw flags from KnownBits (e.g., `trunc (ashr i64 X, 32) to 
i32`). BTW we never set poison-generating flags in `SimplifyDemanded`.


https://github.com/llvm/llvm-project/pull/87910
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [llvm] [InstCombine] Infer nsw/nuw for trunc (PR #87910)

[Nikita Popov via cfe-commits]

@@ -897,7 +897,20 @@ Instruction *InstCombinerImpl::visitTrunc(TruncInst 
) {
 }
   }
 
-  return nullptr;
+  bool Changed = false;
+  if (!Trunc.hasNoSignedWrap() &&
+  ComputeMaxSignificantBits(Src, /*Depth=*/0, ) <= DestWidth) {
+Trunc.setHasNoSignedWrap(true);
+Changed = true;
+  }
+  if (!Trunc.hasNoUnsignedWrap() &&
+  MaskedValueIsZero(Src, APInt::getBitsSetFrom(SrcWidth, DestWidth),
+/*Depth=*/0, )) {
+Trunc.setHasNoUnsignedWrap(true);
+Changed = true;

nikic wrote:

Can we infer this in SimplifyDemanded instead, where we already have the 
KnownBits of the trunc arg?

https://github.com/llvm/llvm-project/pull/87910
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [llvm] [InstCombine] Infer nsw/nuw for trunc (PR #87910)

[via cfe-commits]

goldsteinn wrote:

LGTM.

https://github.com/llvm/llvm-project/pull/87910
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [llvm] [InstCombine] Infer nsw/nuw for trunc (PR #87910)

[via cfe-commits]

goldsteinn wrote:

Not an issue now but I think we should look into updating 
`computeKnownBitsFromContext` to look use `trunc nsw/nuw` uses of `X`. If the 
use is dominating / noundef, we can infer bits about `X`.
Not really an issue if we only use `KnownBits` to infer `nuw`/`nsw`, but once 
we add support in `SCEV`/`CVP`/`SCCP`/etc.. it may be useful.

Guess same is true for `zext nneg` now (and could probably also do `sub nuw X, 
nonzero_Y` now in `isKnownNonNullFromDominatingCondition`).

https://github.com/llvm/llvm-project/pull/87910
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [llvm] [InstCombine] Infer nsw/nuw for trunc (PR #87910)

[via cfe-commits]

@@ -897,7 +897,20 @@ Instruction *InstCombinerImpl::visitTrunc(TruncInst 
) {
 }
   }
 
-  return nullptr;
+  bool Changed = false;
+  if (!Trunc.hasNoSignedWrap() &&
+  ComputeMaxSignificantBits(Src, /*Depth=*/0, ) <= DestWidth) {
+Trunc.setHasNoSignedWrap(true);
+Changed = true;
+  }
+  if (!Trunc.hasNoUnsignedWrap() &&
+  MaskedValueIsZero(Src, APInt::getBitsSetFrom(SrcWidth, DestWidth),
+/*Depth=*/0, )) {
+Trunc.setHasNoUnsignedWrap(true);
+Changed = true;
+  }

goldsteinn wrote:

>From an efficiency POV, I think you should probably startout with a 
>`computeKnownBits` on src and see if you get lucky about leading ones / zeros. 
>Then if you don't, do a call to `computeMaxSignificantBits`.

https://github.com/llvm/llvm-project/pull/87910
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits