[sil-opt] Fix covering all-case of trapping instruction of SILInstruction::mayTrap

[freddi-kit]

Hi, I found that the function which checking trapping SIL instruction may have an issue, so I want to fix by this PR.
(moved from #33830 to fix commit comment)

Abstract

Making SILInstruction:: mayTrap covering all cases of trapping instruction.

Issue

Having no issue-report

Description

It seems that SILInstruction::mayTrap() is checking that the instruction may trap or not.

And, according to the sil.rst, there are 4 instruction may trap (runtime-failure).

• cond_fail
• unconditional_checked_cast
• unconditional_checked_cast_addr
• unconditional_checked_cast_value

ref: https://github.com/apple/swift/blob/master/docs/SIL.rst#cond_fail and https://github.com/apple/swift/blob/master/docs/SIL.rst#checked-conversions

However, unconditional_checked_cast_value is missed on mayTrap case in current implementation so this commit added it to cover all-case.

This missing case has a risk to remove instruction which should be marked as side-effect, on like Dead Code Elimination, so should be fixed

[freddi-kit]

If the current one is correct, I think it would be better to add some comments of it to source code, what do you think?

[eeckstein]

Can you add a test case in https://github.com/apple/swift/blob/master/test/SILOptimizer/side-effect.sil
As an example, see the test function "sil @checkedcast".

Otherwise LGTM!

[freddi-kit]

Oh, sorry for not adding test, let me add it! 👍

[freddi-kit]

@eeckstein

I added test of unconditional_checked_cast_value with missing unconditional_checked_cast_addr at 2676839.

If no unconditional_checked_cast_value case in mayTrap, test result is <func=r,param0=>. But now it will be <func=r,param0=;trap> to show it as trap. Please check it.

[eeckstein]

LGTM, thanks!

[freddi-kit]

Thank you for reviewing!

[freddi-kit]

Should I run test?

[theblixguy]

@swift-ci please test

[theblixguy]

@swift-ci please test macOS

[theblixguy]

@swift-ci please test Windows

[theblixguy]

@swift-ci please test macOS

[swift-ci]

Build failed
Swift Test OS X Platform
Git Sha - 2676839

[theblixguy]

macOS failure should be fixed once swiftlang/swift-package-manager#2924 is merged.

[xedin]

@swift-ci please test macOS platform