Bug 101608 - ranges::fill/fill_n missing std::is_constant_evaluated() condition for __builtin_memset
Summary: ranges::fill/fill_n missing std::is_constant_evaluated() condition for __buil...
Status: RESOLVED FIXED
Alias: None
Product: gcc
Classification: Unclassified
Component: libstdc++ (show other bugs)
Version: 12.0
: P3 normal
Target Milestone: 10.4
Assignee: Not yet assigned to anyone
URL:
Keywords: rejects-valid
Depends on:
Blocks:
 
Reported: 2021-07-24 04:00 UTC by 康桓瑋
Modified: 2021-11-26 14:28 UTC (History)
1 user (show)

See Also:
Host:
Target:
Build:
Known to work:
Known to fail:
Last reconfirmed: 2021-11-23 00:00:00


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description 康桓瑋 2021-07-24 04:00:51 UTC
ranges_algobase.h#L529:

    if constexpr (is_pointer_v<_Out>
    	            // Note that __is_byte already implies !is_volatile.
    	            && __is_byte<remove_pointer_t<_Out>>::__value
    	            && integral<_Tp>)
    {
      __builtin_memset(__first, static_cast<unsigned char>(__value), __n);
      return __first + __n;
    }


We should ensure that std::is_constant_evaluated() is false before calling __builtin_memset since it is not usable in constexpr contexts.


#include <algorithm>

constexpr auto unused = [] {
  std::array<unsigned char, 5> r{};
  std::ranges::fill(r, 0);
  return 0;
}();

https://godbolt.org/z/bnYxY78o8
Comment 1 康桓瑋 2021-08-02 17:14:37 UTC
This may help:

--- a/ranges_algobase.h
+++ b/ranges_algobase.h
@@ -525,16 +525,22 @@ namespace ranges
 	if (__n <= 0)
 	  return __first;
 
-	// TODO: Generalize this optimization to contiguous iterators.
-	if constexpr (is_pointer_v<_Out>
-		      // Note that __is_byte already implies !is_volatile.
-		      && __is_byte<remove_pointer_t<_Out>>::__value
-		      && integral<_Tp>)
+#ifdef __cpp_lib_is_constant_evaluated
+	if (!std::is_constant_evaluated())
+#endif
 	  {
-	    __builtin_memset(__first, static_cast<unsigned char>(__value), __n);
-	    return __first + __n;
+	    // TODO: Generalize this optimization to contiguous iterators.
+	    if constexpr (is_pointer_v<_Out>
+	            // Note that __is_byte already implies !is_volatile.
+	            && __is_byte<remove_pointer_t<_Out>>::__value
+	            && integral<_Tp>)
+	      {
+	        __builtin_memset(__first, static_cast<unsigned char>(__value), __n);
+	        return __first + __n;
+	      }
 	  }
-	else if constexpr (is_scalar_v<_Tp>)
+	
+	if constexpr (is_scalar_v<_Tp>)
 	  {
 	    const auto __tmp = __value;
 	    for (; __n > 0; --__n, (void)++__first)
Comment 2 GCC Commits 2021-11-25 20:03:43 UTC
The master branch has been updated by Jonathan Wakely <redi@gcc.gnu.org>:

https://gcc.gnu.org/g:82c3657dd74896b39937bb0a2aaeba9b8ca105fd

commit r12-5530-g82c3657dd74896b39937bb0a2aaeba9b8ca105fd
Author: Jonathan Wakely <jwakely@redhat.com>
Date:   Wed Nov 24 13:17:54 2021 +0000

    libstdc++: Do not use memset in constexpr calls to ranges::fill_n [PR101608]
    
    libstdc++-v3/ChangeLog:
    
            PR libstdc++/101608
            * include/bits/ranges_algobase.h (__fill_n_fn): Check for
            constant evaluation before using memset.
            * testsuite/25_algorithms/fill_n/constrained.cc: Check
            byte-sized values as well.
Comment 3 GCC Commits 2021-11-25 23:07:23 UTC
The releases/gcc-11 branch has been updated by Jonathan Wakely <redi@gcc.gnu.org>:

https://gcc.gnu.org/g:7ae6e4e3831429d20eea1be285dbc6a4a005930f

commit r11-9314-g7ae6e4e3831429d20eea1be285dbc6a4a005930f
Author: Jonathan Wakely <jwakely@redhat.com>
Date:   Wed Nov 24 13:17:54 2021 +0000

    libstdc++: Do not use memset in constexpr calls to ranges::fill_n [PR101608]
    
    libstdc++-v3/ChangeLog:
    
            PR libstdc++/101608
            * include/bits/ranges_algobase.h (__fill_n_fn): Check for
            constant evaluation before using memset.
            * testsuite/25_algorithms/fill_n/constrained.cc: Check
            byte-sized values as well.
    
    (cherry picked from commit 82c3657dd74896b39937bb0a2aaeba9b8ca105fd)
Comment 4 GCC Commits 2021-11-26 12:52:43 UTC
The releases/gcc-10 branch has been updated by Jonathan Wakely <redi@gcc.gnu.org>:

https://gcc.gnu.org/g:d0ac292c6083fab4bad79a08d23533f537a885d4

commit r10-10296-gd0ac292c6083fab4bad79a08d23533f537a885d4
Author: Jonathan Wakely <jwakely@redhat.com>
Date:   Wed Nov 24 13:17:54 2021 +0000

    libstdc++: Do not use memset in constexpr calls to ranges::fill_n [PR101608]
    
    libstdc++-v3/ChangeLog:
    
            PR libstdc++/101608
            * include/bits/ranges_algobase.h (__fill_n_fn): Check for
            constant evaluation before using memset.
            * testsuite/25_algorithms/fill_n/constrained.cc: Check
            byte-sized values as well.
    
    (cherry picked from commit 82c3657dd74896b39937bb0a2aaeba9b8ca105fd)
Comment 5 Jonathan Wakely 2021-11-26 14:28:55 UTC
Fixed for 10.4 and 11.3